diff --git a/src/main/ipc/filesystem-download-folder.ts b/src/main/ipc/filesystem-download-folder.ts new file mode 100644 index 000000000..55b1421f3 --- /dev/null +++ b/src/main/ipc/filesystem-download-folder.ts @@ -0,0 +1,108 @@ +import { BrowserWindow, dialog, ipcMain } from 'electron' +import { randomUUID } from 'node:crypto' +import { rm, stat } from 'node:fs/promises' +import { dirname, join } from 'node:path' +import { getRuntimePathBasename } from '../../shared/cross-platform-path' +import { sanitizeLocalDownloadFilename } from '../local-download-filename' +import { promoteLocalDownloadedFolder } from '../local-downloaded-folder-promotion' +import { requireSshFilesystemProvider } from '../providers/ssh-filesystem-dispatch' +import { isENOENT } from './filesystem-auth' + +type DownloadFolderResult = { canceled: true } | { canceled: false; destinationPath: string } + +function validateRequiredString(value: unknown, label: string): string { + if (typeof value !== 'string' || value.trim() === '') { + throw new Error(`${label} is required`) + } + return value +} + +function createSiblingTransferPath(destinationPath: string, suffix: string): string { + // Why: promotion uses rename/no-clobber operations that must stay on the + // destination volume, so transfer paths intentionally remain siblings. + return join(dirname(destinationPath), `.${randomUUID()}.${suffix}`) +} + +async function assertDownloadFolderDestinationAvailable(destinationPath: string): Promise { + try { + await stat(destinationPath) + } catch (error) { + if (isENOENT(error)) { + return + } + throw error + } + throw new Error('Destination folder already exists') +} + +async function cleanupLocalTransferDirectory(dirPath: string): Promise { + try { + await rm(dirPath, { recursive: true, force: true }) + } catch (error) { + // Why: cleanup must not mask the transfer error, but a leaked recursive + // download tree needs enough visibility to diagnose and remove it. + console.warn(`[filesystem] Failed to remove temporary folder download '${dirPath}'`, error) + } +} + +// Why: keep folder-download IPC out of filesystem.ts — that module is already large. +export function registerFilesystemDownloadFolderHandlers(): void { + ipcMain.handle( + 'fs:downloadFolder', + async ( + event, + args: { dirPath?: string; connectionId?: string } + ): Promise => { + const dirPath = validateRequiredString(args?.dirPath, 'dirPath') + const connectionId = validateRequiredString(args?.connectionId, 'connectionId') + const provider = requireSshFilesystemProvider(connectionId) + if (!provider.downloadFolder) { + throw new Error( + 'Remote folder download is unavailable. Reconnect the SSH target and retry.' + ) + } + const abortController = new AbortController() + const abortOnSenderDestroyed = (): void => { + abortController.abort(new Error('Folder download canceled because the window closed')) + } + event.sender.once('destroyed', abortOnSenderDestroyed) + if (event.sender.isDestroyed()) { + abortOnSenderDestroyed() + } + try { + abortController.signal.throwIfAborted() + const remoteBasename = getRuntimePathBasename(dirPath) + const destinationBasename = sanitizeLocalDownloadFilename(remoteBasename) + const parentWindow = BrowserWindow.fromWebContents(event.sender) ?? undefined + // Why: after the local capability/abort checks, open the picker before + // remote tree validation so SSH latency does not delay click feedback. + const dialogOptions: Electron.OpenDialogOptions = { + properties: ['openDirectory', 'createDirectory'] + } + const dialogResult = parentWindow + ? await dialog.showOpenDialog(parentWindow, dialogOptions) + : await dialog.showOpenDialog(dialogOptions) + const destinationParent = dialogResult.filePaths?.[0] + if (dialogResult.canceled || !destinationParent) { + return { canceled: true } + } + abortController.signal.throwIfAborted() + + const destinationPath = join(destinationParent, destinationBasename) + await assertDownloadFolderDestinationAvailable(destinationPath) + + const tempPath = createSiblingTransferPath(destinationPath, 'download') + try { + await provider.downloadFolder(dirPath, tempPath, { signal: abortController.signal }) + abortController.signal.throwIfAborted() + await promoteLocalDownloadedFolder(tempPath, destinationPath, abortController.signal) + return { canceled: false, destinationPath } + } finally { + await cleanupLocalTransferDirectory(tempPath) + } + } finally { + event.sender.removeListener('destroyed', abortOnSenderDestroyed) + } + } + ) +} diff --git a/src/main/ipc/filesystem.test.ts b/src/main/ipc/filesystem.test.ts index c1c319c55..d7e48baee 100644 --- a/src/main/ipc/filesystem.test.ts +++ b/src/main/ipc/filesystem.test.ts @@ -1,11 +1,13 @@ /* eslint-disable max-lines -- Why: filesystem authorization and git/file IPC invariants are exercised end-to-end here, so the scenarios stay together to keep the security boundary readable. */ import path from 'node:path' +import { EventEmitter } from 'node:events' import { beforeEach, describe, expect, it, vi } from 'vitest' const handlers = new Map Promise | unknown>() const { handleMock, showSaveDialogMock, + showOpenDialogMock, fromWebContentsMock, trashItemMock, readdirMock, @@ -43,10 +45,12 @@ const { getSshFilesystemProviderMock, getSshGitProviderMock, tryDeleteWslUncPathMock, - recordCrashBreadcrumbMock + recordCrashBreadcrumbMock, + promoteLocalDownloadedFolderMock } = vi.hoisted(() => ({ handleMock: vi.fn(), showSaveDialogMock: vi.fn(), + showOpenDialogMock: vi.fn(), fromWebContentsMock: vi.fn(), trashItemMock: vi.fn(), readdirMock: vi.fn(), @@ -84,7 +88,8 @@ const { getSshFilesystemProviderMock: vi.fn(), getSshGitProviderMock: vi.fn(), tryDeleteWslUncPathMock: vi.fn(), - recordCrashBreadcrumbMock: vi.fn() + recordCrashBreadcrumbMock: vi.fn(), + promoteLocalDownloadedFolderMock: vi.fn() })) vi.mock('electron', () => ({ @@ -92,7 +97,8 @@ vi.mock('electron', () => ({ fromWebContents: fromWebContentsMock }, dialog: { - showSaveDialog: showSaveDialogMock + showSaveDialog: showSaveDialogMock, + showOpenDialog: showOpenDialogMock }, ipcMain: { handle: handleMock @@ -122,6 +128,10 @@ vi.mock('../crash-reporting/crash-breadcrumb-store', () => ({ recordCrashBreadcrumb: recordCrashBreadcrumbMock })) +vi.mock('../local-downloaded-folder-promotion', () => ({ + promoteLocalDownloadedFolder: promoteLocalDownloadedFolderMock +})) + vi.mock('../git/status', () => ({ commitChanges: commitChangesMock, getStatus: getStatusMock, @@ -222,6 +232,10 @@ async function withPlatform(platform: NodeJS.Platform, run: () => Promise) } describe('registerFilesystemHandlers', () => { + const folderDownloadSender = Object.assign(new EventEmitter(), { + isDestroyed: vi.fn(() => false) + }) + const folderDownloadEvent = { sender: folderDownloadSender } const store = { getRepos: () => [ { @@ -238,10 +252,13 @@ describe('registerFilesystemHandlers', () => { } beforeEach(() => { + folderDownloadSender.removeAllListeners() + folderDownloadSender.isDestroyed.mockReset().mockReturnValue(false) handlers.clear() for (const mock of [ handleMock, showSaveDialogMock, + showOpenDialogMock, fromWebContentsMock, trashItemMock, readdirMock, @@ -278,7 +295,8 @@ describe('registerFilesystemHandlers', () => { cancelGeneratePullRequestFieldsLocalMock, getSshFilesystemProviderMock, getSshGitProviderMock, - tryDeleteWslUncPathMock + tryDeleteWslUncPathMock, + promoteLocalDownloadedFolderMock ]) { mock.mockReset() } @@ -304,7 +322,9 @@ describe('registerFilesystemHandlers', () => { trashItemMock.mockResolvedValue(undefined) // Default: not a WSL UNC path, so deletePath falls through to shell.trashItem. tryDeleteWslUncPathMock.mockResolvedValue(false) + promoteLocalDownloadedFolderMock.mockResolvedValue(undefined) showSaveDialogMock.mockResolvedValue({ canceled: true }) + showOpenDialogMock.mockResolvedValue({ canceled: true, filePaths: [] }) fromWebContentsMock.mockReturnValue(null) getSshGitProviderMock.mockReturnValue(null) statMock.mockResolvedValue({ size: 10, isDirectory: () => false, mtimeMs: 123 }) @@ -711,6 +731,246 @@ describe('registerFilesystemHandlers', () => { expect(rmMock).toHaveBeenCalledWith(backupPath, { force: true }) }) + it('downloads remote folders into a temporary sibling before promotion', async () => { + const provider = { + stat: vi.fn().mockResolvedValue({ size: 0, type: 'directory', mtime: 123 }), + downloadFolder: vi.fn().mockResolvedValue(undefined) + } + getSshFilesystemProviderMock.mockReturnValue(provider) + showOpenDialogMock.mockResolvedValue({ canceled: false, filePaths: ['/downloads'] }) + statMock.mockRejectedValue(Object.assign(new Error('missing'), { code: 'ENOENT' })) + registerFilesystemHandlers(store as never) + + await expect( + handlers.get('fs:downloadFolder')!(folderDownloadEvent, { + dirPath: '/remote/src', + connectionId: 'ssh-1' + }) + ).resolves.toEqual({ canceled: false, destinationPath: path.join('/downloads', 'src') }) + + const tempPath = provider.downloadFolder.mock.calls[0][1] + expect(path.dirname(tempPath)).toBe(path.normalize('/downloads')) + expect(showOpenDialogMock).toHaveBeenCalledWith({ + properties: ['openDirectory', 'createDirectory'] + }) + expect(provider.downloadFolder).toHaveBeenCalledWith( + '/remote/src', + tempPath, + expect.objectContaining({ signal: expect.anything() }) + ) + expect(folderDownloadSender.listenerCount('destroyed')).toBe(0) + expect(promoteLocalDownloadedFolderMock).toHaveBeenCalledWith( + tempPath, + path.join('/downloads', 'src'), + expect.anything() + ) + }) + + it('returns canceled remote folder downloads without transferring', async () => { + const provider = { + stat: vi.fn().mockResolvedValue({ size: 0, type: 'directory', mtime: 123 }), + downloadFolder: vi.fn() + } + getSshFilesystemProviderMock.mockReturnValue(provider) + showOpenDialogMock.mockResolvedValue({ canceled: true, filePaths: [] }) + registerFilesystemHandlers(store as never) + + await expect( + handlers.get('fs:downloadFolder')!(folderDownloadEvent, { + dirPath: '/remote/src', + connectionId: 'ssh-1' + }) + ).resolves.toEqual({ canceled: true }) + + expect(provider.downloadFolder).not.toHaveBeenCalled() + }) + + it('aborts before opening the folder picker when the renderer is already destroyed', async () => { + const provider = { + downloadFolder: vi.fn() + } + getSshFilesystemProviderMock.mockReturnValue(provider) + folderDownloadSender.isDestroyed.mockReturnValue(true) + registerFilesystemHandlers(store as never) + + await expect( + handlers.get('fs:downloadFolder')!(folderDownloadEvent, { + dirPath: '/remote/src', + connectionId: 'ssh-1' + }) + ).rejects.toThrow('window closed') + + expect(showOpenDialogMock).not.toHaveBeenCalled() + expect(provider.downloadFolder).not.toHaveBeenCalled() + expect(folderDownloadSender.listenerCount('destroyed')).toBe(0) + }) + + it('opens the folder picker before SSH folder validation', async () => { + const provider = { + stat: vi.fn().mockResolvedValue({ size: 0, type: 'directory', mtime: 123 }), + downloadFolder: vi.fn().mockResolvedValue(undefined) + } + getSshFilesystemProviderMock.mockReturnValue(provider) + showOpenDialogMock.mockImplementation(async () => { + expect(provider.downloadFolder).not.toHaveBeenCalled() + return { canceled: false, filePaths: ['/downloads'] } + }) + statMock.mockRejectedValue(Object.assign(new Error('missing'), { code: 'ENOENT' })) + registerFilesystemHandlers(store as never) + + await expect( + handlers.get('fs:downloadFolder')!(folderDownloadEvent, { + dirPath: '/remote/src', + connectionId: 'ssh-1' + }) + ).resolves.toEqual({ canceled: false, destinationPath: path.join('/downloads', 'src') }) + + expect(provider.downloadFolder).toHaveBeenCalledTimes(1) + }) + + it('rejects remote folder downloads when the destination folder already exists', async () => { + const provider = { + stat: vi.fn().mockResolvedValue({ size: 0, type: 'directory', mtime: 123 }), + downloadFolder: vi.fn() + } + getSshFilesystemProviderMock.mockReturnValue(provider) + showOpenDialogMock.mockResolvedValue({ canceled: false, filePaths: ['/downloads'] }) + statMock.mockResolvedValue({ isDirectory: () => true }) + registerFilesystemHandlers(store as never) + + await expect( + handlers.get('fs:downloadFolder')!(folderDownloadEvent, { + dirPath: '/remote/src', + connectionId: 'ssh-1' + }) + ).rejects.toThrow('Destination folder already exists') + + expect(provider.downloadFolder).not.toHaveBeenCalled() + expect(promoteLocalDownloadedFolderMock).not.toHaveBeenCalled() + }) + + it('rejects remote folder downloads when the remote path is not a directory', async () => { + const provider = { + downloadFolder: vi.fn().mockRejectedValue(new Error('Cannot download a file as a folder')) + } + getSshFilesystemProviderMock.mockReturnValue(provider) + showOpenDialogMock.mockResolvedValue({ canceled: false, filePaths: ['/downloads'] }) + statMock.mockRejectedValue(Object.assign(new Error('missing'), { code: 'ENOENT' })) + registerFilesystemHandlers(store as never) + + await expect( + handlers.get('fs:downloadFolder')!(folderDownloadEvent, { + dirPath: '/remote/file.txt', + connectionId: 'ssh-1' + }) + ).rejects.toThrow('Cannot download a file as a folder') + + expect(provider.downloadFolder).toHaveBeenCalledTimes(1) + expect(promoteLocalDownloadedFolderMock).not.toHaveBeenCalled() + }) + + it('rejects remote folder downloads when the SSH provider cannot transfer folders', async () => { + const provider = { + stat: vi.fn().mockResolvedValue({ size: 0, type: 'directory', mtime: 123 }) + } + getSshFilesystemProviderMock.mockReturnValue(provider) + showOpenDialogMock.mockResolvedValue({ canceled: false, filePaths: ['/downloads'] }) + statMock.mockRejectedValue(Object.assign(new Error('missing'), { code: 'ENOENT' })) + registerFilesystemHandlers(store as never) + + await expect( + handlers.get('fs:downloadFolder')!(folderDownloadEvent, { + dirPath: '/remote/src', + connectionId: 'ssh-1' + }) + ).rejects.toThrow('Remote folder download is unavailable') + + expect(showOpenDialogMock).not.toHaveBeenCalled() + expect(promoteLocalDownloadedFolderMock).not.toHaveBeenCalled() + }) + + it('cleans up a temporary remote folder download when transfer fails', async () => { + const provider = { + stat: vi.fn().mockResolvedValue({ size: 0, type: 'directory', mtime: 123 }), + downloadFolder: vi.fn().mockRejectedValue(new Error('transfer failed')) + } + getSshFilesystemProviderMock.mockReturnValue(provider) + showOpenDialogMock.mockResolvedValue({ canceled: false, filePaths: ['/downloads'] }) + statMock.mockRejectedValue(Object.assign(new Error('missing'), { code: 'ENOENT' })) + registerFilesystemHandlers(store as never) + + await expect( + handlers.get('fs:downloadFolder')!(folderDownloadEvent, { + dirPath: '/remote/src', + connectionId: 'ssh-1' + }) + ).rejects.toThrow('transfer failed') + + const tempPath = provider.downloadFolder.mock.calls[0][1] + expect(promoteLocalDownloadedFolderMock).not.toHaveBeenCalled() + expect(rmMock).toHaveBeenCalledWith(tempPath, { recursive: true, force: true }) + }) + + it('logs a recursive temporary-folder cleanup failure without masking the transfer error', async () => { + const provider = { + downloadFolder: vi.fn().mockRejectedValue(new Error('transfer failed')) + } + const cleanupError = new Error('cleanup denied') + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + getSshFilesystemProviderMock.mockReturnValue(provider) + showOpenDialogMock.mockResolvedValue({ canceled: false, filePaths: ['/downloads'] }) + statMock.mockRejectedValue(Object.assign(new Error('missing'), { code: 'ENOENT' })) + rmMock.mockRejectedValueOnce(cleanupError) + registerFilesystemHandlers(store as never) + + try { + await expect( + handlers.get('fs:downloadFolder')!(folderDownloadEvent, { + dirPath: '/remote/src', + connectionId: 'ssh-1' + }) + ).rejects.toThrow('transfer failed') + + expect(warn).toHaveBeenCalledWith( + expect.stringContaining('Failed to remove temporary folder download'), + cleanupError + ) + } finally { + warn.mockRestore() + } + }) + + it('aborts and cleans up a remote folder download when its renderer closes', async () => { + const provider = { + stat: vi.fn().mockResolvedValue({ size: 0, type: 'directory', mtime: 123 }), + downloadFolder: vi.fn( + (_source: string, _destination: string, options?: { signal?: AbortSignal }) => + new Promise((_resolve, reject) => { + options?.signal?.addEventListener('abort', () => reject(options.signal?.reason), { + once: true + }) + }) + ) + } + getSshFilesystemProviderMock.mockReturnValue(provider) + showOpenDialogMock.mockResolvedValue({ canceled: false, filePaths: ['/downloads'] }) + statMock.mockRejectedValue(Object.assign(new Error('missing'), { code: 'ENOENT' })) + registerFilesystemHandlers(store as never) + + const result = handlers.get('fs:downloadFolder')!(folderDownloadEvent, { + dirPath: '/remote/src', + connectionId: 'ssh-1' + }) + await vi.waitFor(() => expect(provider.downloadFolder).toHaveBeenCalledTimes(1)) + folderDownloadSender.emit('destroyed') + + await expect(result).rejects.toThrow('window closed') + const tempPath = provider.downloadFolder.mock.calls[0][1] + expect(promoteLocalDownloadedFolderMock).not.toHaveBeenCalled() + expect(rmMock).toHaveBeenCalledWith(tempPath, { recursive: true, force: true }) + expect(folderDownloadSender.listenerCount('destroyed')).toBe(0) + }) + it('rejects readFile when the real path escapes allowed roots', async () => { const linkPath = path.resolve('/workspace/repo/link.txt') realpathMock.mockImplementation(async (targetPath: string) => { diff --git a/src/main/ipc/filesystem.ts b/src/main/ipc/filesystem.ts index 931c28c40..aff1a0b65 100644 --- a/src/main/ipc/filesystem.ts +++ b/src/main/ipc/filesystem.ts @@ -122,6 +122,8 @@ import { getRuntimePathBasename } from '../../shared/cross-platform-path' import type { LocalProjectWorktreeGitOptions } from '../project-runtime-git-options' import { registerLocalLogTailHandlers } from './local-log-tail' import { localLogFileIdentity } from '../ai-vault/local-log-tail-reader' +import { sanitizeLocalDownloadFilename } from '../local-download-filename' +import { registerFilesystemDownloadFolderHandlers } from './filesystem-download-folder' import { createSenderScopedRequestCancellations } from './sender-scoped-request-cancellation' // Why: Monaco has large-file optimizations like VS Code; blocking at 5MB makes @@ -148,9 +150,6 @@ const PREVIEWABLE_BINARY_MIME_TYPES: Record = { '.ico': 'image/x-icon', '.pdf': 'application/pdf' } -const WINDOWS_RESERVED_LOCAL_BASENAME = /^(?:con|prn|aux|nul|com[1-9]|lpt[1-9])(?:\..*)?$/i -const LOCAL_FILENAME_REPLACEMENT_CHARS = new Set(['<', '>', ':', '"', '/', '\\', '|', '?', '*']) - async function readLocalLogSnapshot(filePath: string): Promise<{ content: string isBinary: boolean @@ -192,18 +191,6 @@ function validateRequiredString(value: unknown, label: string): string { return value } -function sanitizeSaveDialogFilename(remoteBasename: string): string { - const sanitized = Array.from(remoteBasename, (char) => - char.charCodeAt(0) < 32 || LOCAL_FILENAME_REPLACEMENT_CHARS.has(char) ? '_' : char - ) - .join('') - .replace(/[. ]+$/g, '') - if (!sanitized || WINDOWS_RESERVED_LOCAL_BASENAME.test(sanitized)) { - return 'download' - } - return sanitized -} - function decodeDownloadedFileContent(content: string, encoding: 'utf8' | 'base64'): Buffer { if (encoding === 'base64') { return Buffer.from(content, 'base64') @@ -223,6 +210,8 @@ type DownloadSession = { const DOWNLOAD_SESSION_TTL_MS = 30 * 60 * 1000 function createSiblingTransferPath(destinationPath: string, suffix: string): string { + // Why: promotion uses rename/no-clobber operations that must stay on the + // destination volume, so transfer paths intentionally remain siblings. return join(dirname(destinationPath), `.${randomUUID()}.${suffix}`) } @@ -641,7 +630,7 @@ export function registerFilesystemHandlers( } const remoteBasename = getRuntimePathBasename(filePath) - const defaultPath = sanitizeSaveDialogFilename(remoteBasename) + const defaultPath = sanitizeLocalDownloadFilename(remoteBasename) const parentWindow = BrowserWindow.fromWebContents(event.sender) ?? undefined const dialogResult = parentWindow ? await dialog.showSaveDialog(parentWindow, { defaultPath }) @@ -667,13 +656,15 @@ export function registerFilesystemHandlers( } ) + registerFilesystemDownloadFolderHandlers() + ipcMain.handle( 'fs:saveDownloadedFile', async ( event, args: { suggestedName?: string; content?: string; encoding?: 'utf8' | 'base64' } ): Promise => { - const suggestedName = sanitizeSaveDialogFilename( + const suggestedName = sanitizeLocalDownloadFilename( validateRequiredString(args?.suggestedName, 'suggestedName') ) if (typeof args?.content !== 'string') { @@ -719,7 +710,7 @@ export function registerFilesystemHandlers( destinationPath: string } > => { - const suggestedName = sanitizeSaveDialogFilename( + const suggestedName = sanitizeLocalDownloadFilename( validateRequiredString(args?.suggestedName, 'suggestedName') ) const parentWindow = BrowserWindow.fromWebContents(event.sender) ?? undefined diff --git a/src/main/ipc/ssh.test.ts b/src/main/ipc/ssh.test.ts index aaeb33f5b..c925f592a 100644 --- a/src/main/ipc/ssh.test.ts +++ b/src/main/ipc/ssh.test.ts @@ -546,6 +546,7 @@ describe('SSH IPC handlers', () => { status: 'connected', error: null, reconnectAttempt: 0, + supportsFolderDownload: true, remotePlatform: 'win32' } }) @@ -603,7 +604,8 @@ describe('SSH IPC handlers', () => { targetId: 'ssh-1', status: 'connected', error: null, - reconnectAttempt: 0 + reconnectAttempt: 0, + supportsFolderDownload: true } }) expect(handlers.get('ssh:getState')!(null, { targetId: 'ssh-1' })).toEqual({ diff --git a/src/main/ipc/ssh.ts b/src/main/ipc/ssh.ts index 9526303a0..ee2c0752c 100644 --- a/src/main/ipc/ssh.ts +++ b/src/main/ipc/ssh.ts @@ -278,6 +278,12 @@ function clearRelayStateOverride(targetId: string): void { relayStateOverrides.delete(targetId) } +function connectionSupportsFolderDownload(targetId: string): boolean { + // Why: ready legacy/test connections without an explicit transport are ssh2-shaped; + // only a confirmed system-SSH transport must remove the SFTP-only capability. + return connectionManager?.getConnection(targetId)?.usesSystemSshTransport?.() !== true +} + function getPublicSshState(targetId: string): SshConnectionState | undefined { const state = relayStateOverrides.get(targetId) ?? connectionManager!.getState(targetId) return state ? withSshRemotePlatform(targetId, state) : undefined @@ -674,7 +680,8 @@ function configureRelaySessionCallbacks(session: SshRelaySession): void { targetId: tid, status: 'connected', error: null, - reconnectAttempt: 0 + reconnectAttempt: 0, + supportsFolderDownload: connectionSupportsFolderDownload(tid) }) } void restorePortForwards(tid, getCurrentMainWindow) @@ -925,7 +932,8 @@ export function registerSshHandlers( targetId, status: 'connected', error: null, - reconnectAttempt: 0 + reconnectAttempt: 0, + supportsFolderDownload: conn.usesSystemSshTransport?.() !== true }) } catch (err) { // Relay deployment failed — disconnect SSH diff --git a/src/main/local-download-filename.ts b/src/main/local-download-filename.ts new file mode 100644 index 000000000..3e128ef12 --- /dev/null +++ b/src/main/local-download-filename.ts @@ -0,0 +1,15 @@ +const WINDOWS_RESERVED_LOCAL_BASENAME = + /^(?:con|prn|aux|nul|conin\$|conout\$|com[1-9¹²³]|lpt[1-9¹²³])(?:\..*)?$/i +const LOCAL_FILENAME_REPLACEMENT_CHARS = new Set(['<', '>', ':', '"', '/', '\\', '|', '?', '*']) + +export function sanitizeLocalDownloadFilename(remoteBasename: string): string { + const sanitized = Array.from(remoteBasename, (char) => + char.charCodeAt(0) < 32 || LOCAL_FILENAME_REPLACEMENT_CHARS.has(char) ? '_' : char + ) + .join('') + .replace(/[. ]+$/g, '') + if (!sanitized || WINDOWS_RESERVED_LOCAL_BASENAME.test(sanitized)) { + return 'download' + } + return sanitized +} diff --git a/src/main/local-downloaded-folder-promotion.test.ts b/src/main/local-downloaded-folder-promotion.test.ts new file mode 100644 index 000000000..57d82edd6 --- /dev/null +++ b/src/main/local-downloaded-folder-promotion.test.ts @@ -0,0 +1,218 @@ +import { afterEach, describe, expect, it } from 'vitest' +import { writeFileSync } from 'node:fs' +import { + lstat, + mkdtemp, + mkdir, + open, + readFile, + readdir, + rm, + symlink, + writeFile +} from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' + +import { + promoteLocalDownloadedFolder, + copyLocalDownloadedFileNoClobber, + publishLocalDownloadedFileNoClobber +} from './local-downloaded-folder-promotion' + +describe('promoteLocalDownloadedFolder', () => { + const roots: string[] = [] + + afterEach(async () => { + await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true }))) + }) + + async function createPaths(): Promise<{ + root: string + tempPath: string + destinationPath: string + }> { + const root = await mkdtemp(join(tmpdir(), 'orca-folder-promotion-')) + roots.push(root) + const tempPath = join(root, '.transfer.download') + const destinationPath = join(root, 'downloaded') + await mkdir(join(tempPath, 'nested'), { recursive: true }) + await writeFile(join(tempPath, 'nested', 'file.txt'), 'remote') + return { root, tempPath, destinationPath } + } + + it('claims the destination and promotes the completed temporary tree', async () => { + const { root, tempPath, destinationPath } = await createPaths() + + await promoteLocalDownloadedFolder(tempPath, destinationPath) + + await expect(readFile(join(destinationPath, 'nested', 'file.txt'), 'utf8')).resolves.toBe( + 'remote' + ) + await expect(readdir(root)).resolves.toEqual(['downloaded']) + }) + + it('does not replace a destination created before publication', async () => { + const { tempPath, destinationPath } = await createPaths() + await mkdir(destinationPath) + await writeFile(join(destinationPath, 'local.txt'), 'local') + + await expect(promoteLocalDownloadedFolder(tempPath, destinationPath)).rejects.toThrow( + 'Destination folder already exists' + ) + + await expect(readFile(join(destinationPath, 'local.txt'), 'utf8')).resolves.toBe('local') + await expect(readFile(join(tempPath, 'nested', 'file.txt'), 'utf8')).resolves.toBe('remote') + }) + + it('does not replace a file added inside a claimed destination', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-folder-promotion-')) + roots.push(root) + const sourcePath = join(root, 'remote.txt') + const destinationPath = join(root, 'third-party.txt') + await writeFile(sourcePath, 'remote') + await writeFile(destinationPath, 'third-party') + + await expect( + publishLocalDownloadedFileNoClobber(sourcePath, destinationPath) + ).rejects.toMatchObject({ code: 'EEXIST' }) + + await expect(readFile(sourcePath, 'utf8')).resolves.toBe('remote') + await expect(readFile(destinationPath, 'utf8')).resolves.toBe('third-party') + }) + + it('preserves a mutation made after hard-link state is recorded', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-folder-promotion-')) + roots.push(root) + const sourcePath = join(root, 'remote.txt') + const destinationPath = join(root, 'published.txt') + await writeFile(sourcePath, 'remote') + const signal = { + throwIfAborted: () => { + writeFileSync(destinationPath, 'third-party mutation') + throw new Error('window closed') + } + } as unknown as AbortSignal + + await expect( + publishLocalDownloadedFileNoClobber(sourcePath, destinationPath, signal) + ).rejects.toThrow('window closed') + + await expect(readFile(sourcePath, 'utf8')).resolves.toBe('third-party mutation') + await expect(readFile(destinationPath, 'utf8')).resolves.toBe('third-party mutation') + }) + + it('does not claim a destination after cancellation', async () => { + const { tempPath, destinationPath } = await createPaths() + const controller = new AbortController() + controller.abort(new Error('window closed')) + + await expect( + promoteLocalDownloadedFolder(tempPath, destinationPath, controller.signal) + ).rejects.toThrow('window closed') + + await expect(readdir(tempPath)).resolves.toEqual(['nested']) + await expect(readdir(destinationPath)).rejects.toMatchObject({ code: 'ENOENT' }) + }) + + it('rolls back unchanged entries after a mid-publication failure', async () => { + const { tempPath, destinationPath } = await createPaths() + await writeFile(join(tempPath, 'a-first.txt'), 'remote') + await symlink('a-first.txt', join(tempPath, 'z-unsupported-link')) + + await expect(promoteLocalDownloadedFolder(tempPath, destinationPath)).rejects.toThrow( + "Unexpected local download entry 'z-unsupported-link'" + ) + + await expect(readdir(destinationPath)).rejects.toMatchObject({ code: 'ENOENT' }) + }) + + it('preserves a third-party file mutation when later publication fails', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-folder-promotion-')) + roots.push(root) + const tempPath = join(root, '.transfer.download') + const destinationPath = join(root, 'downloaded') + await mkdir(join(tempPath, 'b-work'), { recursive: true }) + await writeFile(join(tempPath, 'a-first.txt'), 'remote') + await Promise.all( + Array.from({ length: 200 }, (_, index) => + writeFile(join(tempPath, 'b-work', `${String(index).padStart(3, '0')}.txt`), 'remote') + ) + ) + await symlink('a-first.txt', join(tempPath, 'z-unsupported-link')) + + const promotion = promoteLocalDownloadedFolder(tempPath, destinationPath) + const failure = expect(promotion).rejects.toThrow( + "Unexpected local download entry 'z-unsupported-link'" + ) + const publishedFile = join(destinationPath, 'a-first.txt') + for (;;) { + try { + await readFile(publishedFile) + break + } catch { + await new Promise((resolve) => setImmediate(resolve)) + } + } + await writeFile(publishedFile, 'third-party mutation') + + await failure + await expect(readFile(publishedFile, 'utf8')).resolves.toBe('third-party mutation') + }) + + it('cancels between entries and rolls back unchanged published files', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-folder-promotion-')) + roots.push(root) + const tempPath = join(root, '.transfer.download') + const destinationPath = join(root, 'downloaded') + await mkdir(tempPath) + await Promise.all( + Array.from({ length: 300 }, (_, index) => + writeFile(join(tempPath, `${String(index).padStart(3, '0')}.txt`), 'remote') + ) + ) + const controller = new AbortController() + + const promotion = promoteLocalDownloadedFolder(tempPath, destinationPath, controller.signal) + const failure = expect(promotion).rejects.toThrow('window closed') + for (;;) { + try { + await readFile(join(destinationPath, '000.txt')) + break + } catch { + await new Promise((resolve) => setImmediate(resolve)) + } + } + controller.abort(new Error('window closed')) + + await failure + await expect(readdir(destinationPath)).rejects.toMatchObject({ code: 'ENOENT' }) + }) + + it('cancels a chunked exclusive-copy fallback and removes its partial file', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-folder-promotion-')) + roots.push(root) + const sourcePath = join(root, 'large-source.bin') + const destinationPath = join(root, 'large-destination.bin') + const sourceHandle = await open(sourcePath, 'wx') + await sourceHandle.truncate(64 * 1024 * 1024) + await sourceHandle.close() + const controller = new AbortController() + + const copying = copyLocalDownloadedFileNoClobber(sourcePath, destinationPath, controller.signal) + const failure = expect(copying).rejects.toThrow('copy canceled') + for (;;) { + try { + await lstat(destinationPath) + break + } catch { + await new Promise((resolve) => setImmediate(resolve)) + } + } + controller.abort(new Error('copy canceled')) + + await failure + await expect(lstat(destinationPath)).rejects.toMatchObject({ code: 'ENOENT' }) + await expect(lstat(sourcePath)).resolves.toMatchObject({ size: 64 * 1024 * 1024 }) + }) +}) diff --git a/src/main/local-downloaded-folder-promotion.ts b/src/main/local-downloaded-folder-promotion.ts new file mode 100644 index 000000000..f5ba45f7a --- /dev/null +++ b/src/main/local-downloaded-folder-promotion.ts @@ -0,0 +1,268 @@ +import { fstatSync, lstatSync } from 'node:fs' +import { link, lstat, mkdir, open, readdir, rm, rmdir, unlink } from 'node:fs/promises' +import type { BigIntStats } from 'node:fs' +import type { FileHandle } from 'node:fs/promises' +import { join } from 'node:path' + +const LOCAL_COPY_CHUNK_BYTES = 1024 * 1024 + +type PublishedEntry = { + kind: 'directory' | 'file' + path: string + identity: Pick + fileState?: Pick +} + +function isEEXIST(error: unknown): boolean { + return ( + error instanceof Error && 'code' in error && (error as NodeJS.ErrnoException).code === 'EEXIST' + ) +} + +function hasSameIdentity(current: BigIntStats, published: PublishedEntry): boolean { + const { identity } = published + const hasStableInode = identity.dev !== 0n || identity.ino !== 0n + if (hasStableInode) { + return current.dev === identity.dev && current.ino === identity.ino + } + return identity.birthtimeNs !== 0n && current.birthtimeNs === identity.birthtimeNs +} + +function hasSamePublishedFileState(current: BigIntStats, published: PublishedEntry): boolean { + const state = published.fileState + return ( + state === undefined || + (current.size === state.size && + current.mtimeNs === state.mtimeNs && + current.mode === state.mode) + ) +} + +function publishedEntryFromStats( + kind: PublishedEntry['kind'], + filePath: string, + stats: BigIntStats +): PublishedEntry { + return { + kind, + path: filePath, + identity: { dev: stats.dev, ino: stats.ino, birthtimeNs: stats.birthtimeNs }, + ...(kind === 'file' + ? { + fileState: { + size: stats.size, + mtimeNs: stats.mtimeNs, + mode: stats.mode + } + } + : {}) + } +} + +function updatePublishedFileState(entry: PublishedEntry, stats: BigIntStats): void { + Object.assign(entry, publishedEntryFromStats('file', entry.path, stats)) +} + +async function closeFileHandle(handle: FileHandle | undefined): Promise { + await handle?.close().catch(() => {}) +} + +async function copyTrackedLocalDownloadedFileNoClobber( + sourcePath: string, + destinationPath: string, + publishedEntries: PublishedEntry[], + signal?: AbortSignal +): Promise { + let sourceHandle: FileHandle | undefined + let destinationHandle: FileHandle | undefined + let record: PublishedEntry | undefined + try { + sourceHandle = await open(sourcePath, 'r') + destinationHandle = await open(destinationPath, 'wx') + record = publishedEntryFromStats( + 'file', + destinationPath, + fstatSync(destinationHandle.fd, { bigint: true }) + ) + publishedEntries.push(record) + const buffer = Buffer.allocUnsafe(LOCAL_COPY_CHUNK_BYTES) + let position = 0 + for (;;) { + signal?.throwIfAborted() + const { bytesRead } = await sourceHandle.read(buffer, 0, buffer.length, position) + if (bytesRead === 0) { + break + } + let written = 0 + while (written < bytesRead) { + signal?.throwIfAborted() + const result = await destinationHandle.write( + buffer, + written, + bytesRead - written, + position + written + ) + written += result.bytesWritten + } + position += bytesRead + } + updatePublishedFileState(record, await destinationHandle.stat({ bigint: true })) + } catch (error) { + if (record && destinationHandle) { + await destinationHandle + .stat({ bigint: true }) + .then((stats) => updatePublishedFileState(record!, stats)) + .catch(() => {}) + } + throw error + } finally { + await Promise.all([closeFileHandle(sourceHandle), closeFileHandle(destinationHandle)]) + } + await unlink(sourcePath) +} + +async function publishTrackedFileNoClobber( + sourcePath: string, + destinationPath: string, + publishedEntries: PublishedEntry[], + signal?: AbortSignal +): Promise { + const sourceStats = await lstat(sourcePath, { bigint: true }) + const hardLinkRecord = publishedEntryFromStats('file', destinationPath, sourceStats) + try { + await link(sourcePath, destinationPath) + } catch (error) { + if (isEEXIST(error)) { + throw error + } + await copyTrackedLocalDownloadedFileNoClobber( + sourcePath, + destinationPath, + publishedEntries, + signal + ) + return + } + // Why: hard links preserve dev+ino; register that known ownership first, + // then synchronously snapshot state without an async third-party window. + publishedEntries.push(hardLinkRecord) + updatePublishedFileState(hardLinkRecord, lstatSync(destinationPath, { bigint: true })) + signal?.throwIfAborted() + await unlink(sourcePath) +} + +export async function copyLocalDownloadedFileNoClobber( + sourcePath: string, + destinationPath: string, + signal?: AbortSignal +): Promise { + const publishedEntries: PublishedEntry[] = [] + try { + await copyTrackedLocalDownloadedFileNoClobber( + sourcePath, + destinationPath, + publishedEntries, + signal + ) + } catch (error) { + await rollbackPublishedEntries(publishedEntries) + throw error + } +} + +export async function publishLocalDownloadedFileNoClobber( + sourcePath: string, + destinationPath: string, + signal?: AbortSignal +): Promise { + const publishedEntries: PublishedEntry[] = [] + try { + await publishTrackedFileNoClobber(sourcePath, destinationPath, publishedEntries, signal) + } catch (error) { + await rollbackPublishedEntries(publishedEntries) + throw error + } +} + +async function rollbackPublishedEntries(entries: PublishedEntry[]): Promise { + for (const entry of entries.toReversed()) { + try { + const current = await lstat(entry.path, { bigint: true }) + if (!hasSameIdentity(current, entry)) { + continue + } + if (entry.kind === 'file') { + if (hasSamePublishedFileState(current, entry)) { + await unlink(entry.path) + } + } else { + // Why: rmdir removes only our still-empty directory; third-party children + // make it fail closed instead of being deleted recursively. + await rmdir(entry.path).catch(() => {}) + } + } catch { + // A missing or unreadable path is no longer safe for rollback to own. + } + } +} + +async function publishDirectoryNoClobber( + sourcePath: string, + destinationPath: string, + publishedEntries: PublishedEntry[], + signal?: AbortSignal +): Promise { + await mkdir(destinationPath, { recursive: false }) + // Why: capture the directory identity synchronously after the exclusive + // claim so no async gap exists before rollback ownership is registered. + const destinationStats = lstatSync(destinationPath, { bigint: true }) + publishedEntries.push(publishedEntryFromStats('directory', destinationPath, destinationStats)) + const entries = (await readdir(sourcePath, { withFileTypes: true })).toSorted((a, b) => + a.name.localeCompare(b.name) + ) + for (const entry of entries) { + signal?.throwIfAborted() + const sourceEntryPath = join(sourcePath, entry.name) + const destinationEntryPath = join(destinationPath, entry.name) + if (entry.isDirectory()) { + await publishDirectoryNoClobber( + sourceEntryPath, + destinationEntryPath, + publishedEntries, + signal + ) + } else if (entry.isFile()) { + await publishTrackedFileNoClobber( + sourceEntryPath, + destinationEntryPath, + publishedEntries, + signal + ) + } else { + throw new Error(`Unexpected local download entry '${entry.name}'`) + } + signal?.throwIfAborted() + } + signal?.throwIfAborted() +} + +export async function promoteLocalDownloadedFolder( + tempPath: string, + destinationPath: string, + signal?: AbortSignal +): Promise { + signal?.throwIfAborted() + const publishedEntries: PublishedEntry[] = [] + try { + // Why: Node has no portable atomic no-replace directory rename. Claiming + // the destination first preserves no-clobber while promotion stays local. + await publishDirectoryNoClobber(tempPath, destinationPath, publishedEntries, signal) + } catch (error) { + await rollbackPublishedEntries(publishedEntries) + if (isEEXIST(error)) { + throw new Error('Destination folder already exists') + } + throw error + } + await rm(tempPath, { recursive: true, force: true }).catch(() => {}) +} diff --git a/src/main/providers/ssh-filesystem-download.test.ts b/src/main/providers/ssh-filesystem-download.test.ts new file mode 100644 index 000000000..d65735cb8 --- /dev/null +++ b/src/main/providers/ssh-filesystem-download.test.ts @@ -0,0 +1,248 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' + +import { downloadFolderViaSftp } from './ssh-filesystem-download' + +type SftpEntryKind = 'directory' | 'file' | 'symlink' | 'fifo' + +function sftpStats(kind: SftpEntryKind) { + return { + size: 0, + mode: 0, + uid: 0, + gid: 0, + atime: 0, + mtime: 0, + isDirectory: () => kind === 'directory', + isFile: () => kind === 'file', + isSymbolicLink: () => kind === 'symlink', + isFIFO: () => kind === 'fifo', + isSocket: () => false, + isBlockDevice: () => false, + isCharacterDevice: () => false + } +} + +function sftpEntry(filename: string, kind: SftpEntryKind) { + return { filename, longname: filename, attrs: sftpStats(kind) } +} + +describe('downloadFolderViaSftp', () => { + const roots: string[] = [] + + afterEach(async () => { + await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true }))) + }) + + async function createDestination(): Promise { + const root = await mkdtemp(join(tmpdir(), 'orca-ssh-folder-download-')) + roots.push(root) + return join(root, 'src') + } + + it('uses exclusive local reservations instead of guessing destination case sensitivity', async () => { + const destination = await createDestination() + let transferCount = 0 + const sftp = { + stat: vi.fn((_path: string, callback: (err: Error | undefined, value: unknown) => void) => + callback(undefined, sftpStats('directory')) + ), + readdir: vi.fn((_path: string, callback: (err: Error | undefined, value: unknown) => void) => + callback(undefined, [sftpEntry('A.txt', 'file'), sftpEntry('a.txt', 'file')]) + ), + fastGet: vi.fn((_source: string, _destination: string, callback: (err?: Error) => void) => { + transferCount += 1 + if (transferCount === 1) { + void writeFile(join(destination, 'a.txt'), 'claimed').then(() => callback()) + return + } + callback() + }), + end: vi.fn() + } + + await expect( + downloadFolderViaSftp(async () => sftp as never, '/remote/src', destination) + ).rejects.toThrow("Remote entries map to the same local name 'a.txt'") + expect(sftp.fastGet).toHaveBeenCalledTimes(1) + }) + + it('rejects special remote entries before SFTP tries to open them', async () => { + const destination = await createDestination() + const sftp = { + stat: vi.fn((_path: string, callback: (err: Error | undefined, value: unknown) => void) => + callback(undefined, sftpStats('directory')) + ), + readdir: vi.fn((_path: string, callback: (err: Error | undefined, value: unknown) => void) => + callback(undefined, [sftpEntry('build.pipe', 'fifo')]) + ), + fastGet: vi.fn(), + end: vi.fn() + } + + await expect( + downloadFolderViaSftp(async () => sftp as never, '/remote/src', destination) + ).rejects.toThrow("Cannot download unsupported remote entry 'build.pipe'") + expect(sftp.fastGet).not.toHaveBeenCalled() + }) + + it('rejects a file symlink that could escape the selected remote tree', async () => { + const destination = await createDestination() + const sftp = { + stat: vi.fn( + (remotePath: string, callback: (err: Error | undefined, value: unknown) => void) => + callback(undefined, sftpStats(remotePath === '/remote/src' ? 'directory' : 'file')) + ), + readdir: vi.fn((_path: string, callback: (err: Error | undefined, value: unknown) => void) => + callback(undefined, [sftpEntry('creds', 'symlink')]) + ), + fastGet: vi.fn(), + end: vi.fn() + } + + await expect( + downloadFolderViaSftp(async () => sftp as never, '/remote/src', destination) + ).rejects.toThrow("Cannot download symbolic link 'creds'") + // The link target could be /etc/passwd; rejecting from directory-entry + // metadata means it is never followed with stat or opened by fastGet. + expect(sftp.stat).toHaveBeenCalledTimes(1) + expect(sftp.fastGet).not.toHaveBeenCalled() + }) + + it('sanitizes extended Windows device names in nested entries', async () => { + const destination = await createDestination() + const sftp = { + stat: vi.fn((_path: string, callback: (err: Error | undefined, value: unknown) => void) => + callback(undefined, sftpStats('directory')) + ), + readdir: vi.fn((_path: string, callback: (err: Error | undefined, value: unknown) => void) => + callback(undefined, [sftpEntry('CONIN$', 'file'), sftpEntry('COM¹.txt', 'file')]) + ), + fastGet: vi.fn(), + end: vi.fn() + } + + await expect( + downloadFolderViaSftp(async () => sftp as never, '/remote/src', destination) + ).rejects.toThrow("Remote entries map to the same local name 'download'") + expect(sftp.fastGet).not.toHaveBeenCalled() + }) + + it('preserves legal POSIX backslashes in opaque SFTP child names', async () => { + const destination = await createDestination() + const sourcePath = '/remote/parent\\literal' + const sftp = { + stat: vi.fn((_path: string, callback: (err: Error | undefined, value: unknown) => void) => + callback(undefined, sftpStats('directory')) + ), + readdir: vi.fn((_path: string, callback: (err: Error | undefined, value: unknown) => void) => + callback(undefined, [sftpEntry('..\\secret.txt', 'file')]) + ), + fastGet: vi.fn((_source: string, _destination: string, callback: (err?: Error) => void) => + callback() + ), + end: vi.fn() + } + + await downloadFolderViaSftp(async () => sftp as never, sourcePath, destination, { + windowsRemotePaths: false + }) + + expect(sftp.fastGet).toHaveBeenCalledWith( + '/remote/parent\\literal/..\\secret.txt', + join(destination, '.._secret.txt'), + expect.any(Function) + ) + }) + + it('rejects Windows-path traversal names when the remote host is Windows', async () => { + const destination = await createDestination() + const sftp = { + stat: vi.fn((_path: string, callback: (err: Error | undefined, value: unknown) => void) => + callback(undefined, sftpStats('directory')) + ), + readdir: vi.fn((_path: string, callback: (err: Error | undefined, value: unknown) => void) => + callback(undefined, [sftpEntry('..\\secret.txt', 'file')]) + ), + fastGet: vi.fn(), + end: vi.fn() + } + + await expect( + downloadFolderViaSftp(async () => sftp as never, 'C:/remote/src', destination, { + windowsRemotePaths: true + }) + ).rejects.toThrow("Invalid remote directory entry '..\\secret.txt'") + expect(sftp.fastGet).not.toHaveBeenCalled() + }) + + it('waits for active SFTP file handles to quiesce when canceled', async () => { + const destination = await createDestination() + let fastGetCallback: ((error?: Error) => void) | undefined + const sftp = { + stat: vi.fn((_path: string, callback: (err: Error | undefined, value: unknown) => void) => + callback(undefined, sftpStats('directory')) + ), + readdir: vi.fn((_path: string, callback: (err: Error | undefined, value: unknown) => void) => + callback(undefined, [sftpEntry('first.txt', 'file'), sftpEntry('second.txt', 'file')]) + ), + fastGet: vi.fn((_source: string, _destination: string, callback: (error?: Error) => void) => { + fastGetCallback = callback + }), + end: vi.fn() + } + const controller = new AbortController() + + const result = downloadFolderViaSftp(async () => sftp as never, '/remote/src', destination, { + signal: controller.signal + }) + await vi.waitFor(() => expect(sftp.fastGet).toHaveBeenCalledTimes(1)) + controller.abort(new Error('renderer closed')) + + let settled = false + void result.then( + () => { + settled = true + }, + () => { + settled = true + } + ) + await Promise.resolve() + expect(settled).toBe(false) + fastGetCallback?.(new Error('channel closed')) + + await expect(result).rejects.toThrow('renderer closed') + expect(sftp.fastGet).toHaveBeenCalledTimes(1) + expect(sftp.end).toHaveBeenCalledTimes(1) + }) + + it('cancels a pending SFTP directory read', async () => { + const destination = await createDestination() + let readDirCallback: ((error?: Error) => void) | undefined + const sftp = { + stat: vi.fn((_path: string, callback: (err: Error | undefined, value: unknown) => void) => + callback(undefined, sftpStats('directory')) + ), + readdir: vi.fn((_path: string, callback: (error?: Error) => void) => { + readDirCallback = callback + }), + fastGet: vi.fn(), + end: vi.fn() + } + const controller = new AbortController() + + const result = downloadFolderViaSftp(async () => sftp as never, '/remote/src', destination, { + signal: controller.signal + }) + await vi.waitFor(() => expect(sftp.readdir).toHaveBeenCalledTimes(1)) + controller.abort(new Error('renderer closed')) + readDirCallback?.(new Error('channel closed')) + + await expect(result).rejects.toThrow('renderer closed') + expect(sftp.fastGet).not.toHaveBeenCalled() + expect(sftp.end).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/main/providers/ssh-filesystem-download.ts b/src/main/providers/ssh-filesystem-download.ts new file mode 100644 index 000000000..f73ef4c65 --- /dev/null +++ b/src/main/providers/ssh-filesystem-download.ts @@ -0,0 +1,184 @@ +import { mkdir, open } from 'node:fs/promises' +import { join } from 'node:path' +import type { FileEntryWithStats, SFTPWrapper } from 'ssh2' + +import { + isWindowsAbsolutePathLike, + normalizeRuntimePathSeparators +} from '../../shared/cross-platform-path' +import { sanitizeLocalDownloadFilename } from '../local-download-filename' +import { fastGetViaSftp, readDirViaSftp, statViaSftp } from './ssh-filesystem-provider-sftp' + +export type SftpFactory = (options?: { signal?: AbortSignal }) => Promise + +/** When known, windowsRemotePaths drives remote path joining; omit uses path-shape heuristics. */ +export type FolderDownloadOptions = { signal?: AbortSignal; windowsRemotePaths?: boolean } + +const DOWNLOAD_UNAVAILABLE_MESSAGE = + 'Remote folder download is unavailable. Reconnect the SSH target and retry.' + +function isEEXIST(error: unknown): boolean { + return ( + error instanceof Error && 'code' in error && (error as NodeJS.ErrnoException).code === 'EEXIST' + ) +} + +async function reserveLocalFile(localPath: string, localName: string): Promise { + try { + const handle = await open(localPath, 'wx') + await handle.close() + } catch (error) { + if (isEEXIST(error)) { + throw new Error(`Remote entries map to the same local name '${localName}'`) + } + throw error + } +} + +function remotePathsAreWindows(sourceDir: string, windowsRemotePaths?: boolean): boolean { + // Why: remote path rules belong to the SSH host. Prefer the known host platform + // and only fall back to string shape when the provider could not supply it. + if (windowsRemotePaths !== undefined) { + return windowsRemotePaths + } + return isWindowsAbsolutePathLike(sourceDir) +} + +function joinSftpChildPath( + sourceDir: string, + childName: string, + windowsRemotePaths?: boolean +): string { + const windowsPath = remotePathsAreWindows(sourceDir, windowsRemotePaths) + if ( + !childName || + childName === '.' || + childName === '..' || + childName.includes('/') || + (windowsPath && childName.includes('\\')) + ) { + throw new Error(`Invalid remote directory entry '${childName}'`) + } + const normalizedSource = windowsPath ? normalizeRuntimePathSeparators(sourceDir) : sourceDir + return `${normalizedSource.replace(/\/+$/g, '')}/${childName}` +} + +function classifySftpEntry(entry: FileEntryWithStats): 'directory' | 'file' { + if (entry.attrs.isSymbolicLink()) { + // Why: following either file or directory links can escape the selected tree; + // local symlink creation is also not portable across Orca's supported hosts. + throw new Error(`Cannot download symbolic link '${entry.filename}'`) + } + if (entry.attrs.isDirectory()) { + return 'directory' + } + if (entry.attrs.isFile()) { + return 'file' + } + throw new Error(`Cannot download unsupported remote entry '${entry.filename}'`) +} + +async function downloadDirectoryTree( + sftp: SFTPWrapper, + sourceDir: string, + destinationDir: string, + signal?: AbortSignal, + windowsRemotePaths?: boolean +): Promise { + signal?.throwIfAborted() + const entries = (await readDirViaSftp(sftp, sourceDir, { signal })).filter( + (entry) => entry.filename !== '.' && entry.filename !== '..' + ) + signal?.throwIfAborted() + const usedLocalNames = new Set() + const plannedEntries: { + entry: FileEntryWithStats + kind: 'directory' | 'file' + localName: string + }[] = [] + for (const entry of entries) { + const localName = sanitizeLocalDownloadFilename(entry.filename) + if (usedLocalNames.has(localName)) { + throw new Error(`Remote entries map to the same local name '${localName}'`) + } + usedLocalNames.add(localName) + plannedEntries.push({ + entry, + kind: classifySftpEntry(entry), + localName + }) + } + + await mkdir(destinationDir, { recursive: false }) + for (const { entry, kind, localName } of plannedEntries) { + signal?.throwIfAborted() + const remotePath = joinSftpChildPath(sourceDir, entry.filename, windowsRemotePaths) + const localPath = join(destinationDir, localName) + if (kind === 'directory') { + await downloadDirectoryTree(sftp, remotePath, localPath, signal, windowsRemotePaths) + continue + } + // Why: filesystem semantics belong to the selected volume, not the host OS; + // an exclusive placeholder prevents case/Unicode aliases from overwriting. + await reserveLocalFile(localPath, localName) + await fastGetViaSftp(sftp, remotePath, localPath, { signal }) + } +} + +export async function downloadFileViaSftp( + createSftp: SftpFactory | undefined, + sourcePath: string, + destinationPath: string +): Promise { + if (!createSftp) { + throw new Error('Remote file download is unavailable. Reconnect the SSH target and retry.') + } + const sftp = await createSftp() + try { + await fastGetViaSftp(sftp, sourcePath, destinationPath) + } finally { + sftp.end() + } +} + +export async function downloadFolderViaSftp( + createSftp: SftpFactory | undefined, + sourcePath: string, + destinationPath: string, + options?: FolderDownloadOptions +): Promise { + if (!createSftp) { + throw new Error(DOWNLOAD_UNAVAILABLE_MESSAGE) + } + const signal = options?.signal + signal?.throwIfAborted() + const sftp = await createSftp({ signal }) + let ended = false + const endSftp = (): void => { + if (!ended) { + ended = true + try { + sftp.end() + } catch { + // Why: cleanup is best-effort and must not mask the transfer or abort error. + } + } + } + signal?.addEventListener('abort', endSftp, { once: true }) + try { + const rootStats = await statViaSftp(sftp, sourcePath, { signal }) + if (!rootStats.isDirectory()) { + throw new Error('Cannot download a file as a folder') + } + await downloadDirectoryTree( + sftp, + sourcePath, + destinationPath, + signal, + options?.windowsRemotePaths + ) + } finally { + signal?.removeEventListener('abort', endSftp) + endSftp() + } +} diff --git a/src/main/providers/ssh-filesystem-provider-download-folder.test.ts b/src/main/providers/ssh-filesystem-provider-download-folder.test.ts new file mode 100644 index 000000000..c87530188 --- /dev/null +++ b/src/main/providers/ssh-filesystem-provider-download-folder.test.ts @@ -0,0 +1,173 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { mkdtemp, rm, stat } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' + +import { SshFilesystemProvider } from './ssh-filesystem-provider' + +type SftpEntryKind = 'directory' | 'file' | 'symlink' + +function sftpStats(kind: SftpEntryKind) { + return { + size: 0, + mode: 0, + uid: 0, + gid: 0, + atime: 0, + mtime: 0, + isDirectory: () => kind === 'directory', + isFile: () => kind === 'file', + isSymbolicLink: () => kind === 'symlink', + isFIFO: () => false, + isSocket: () => false, + isBlockDevice: () => false, + isCharacterDevice: () => false + } +} + +function sftpEntry(filename: string, kind: SftpEntryKind) { + return { filename, longname: filename, attrs: sftpStats(kind) } +} + +function createMockMux() { + return { + request: vi.fn(), + onNotification: vi.fn(() => () => {}) + } +} + +describe('SshFilesystemProvider downloadFolder', () => { + let mux: ReturnType + let provider: SshFilesystemProvider + const localDownloadRoots: string[] = [] + + beforeEach(() => { + mux = createMockMux() + provider = new SshFilesystemProvider('conn-1', mux as never) + }) + + afterEach(async () => { + await Promise.all( + localDownloadRoots.splice(0).map((root) => rm(root, { recursive: true, force: true })) + ) + }) + + it('omits folder capability without SFTP while retaining raw file download', async () => { + const downloadFile = vi.fn().mockResolvedValue(undefined) + provider = new SshFilesystemProvider('conn-1', mux as never, undefined, { downloadFile }) + + expect(provider.downloadFolder).toBeUndefined() + await provider.downloadFile('/remote/report.pdf', '/downloads/report.pdf') + expect(downloadFile).toHaveBeenCalledWith('/remote/report.pdf', '/downloads/report.pdf') + }) + + it('downloads a recursive tree through one SFTP session', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-ssh-folder-download-')) + localDownloadRoots.push(root) + const sftp = { + stat: vi.fn( + (remotePath: string, callback: (err: Error | undefined, value: unknown) => void) => + callback(undefined, sftpStats(remotePath === '/remote/src' ? 'directory' : 'file')) + ), + readdir: vi.fn( + (remotePath: string, callback: (err: Error | undefined, value: unknown) => void) => + callback( + undefined, + remotePath === '/remote/src' + ? [sftpEntry('index.ts', 'file'), sftpEntry('lib', 'directory')] + : [sftpEntry('a.ts', 'file')] + ) + ), + fastGet: vi.fn((_source: string, _destination: string, callback: (err?: Error) => void) => + callback() + ), + end: vi.fn() + } + const createSftp = vi.fn(async () => sftp as never) + provider = new SshFilesystemProvider('conn-1', mux as never, createSftp) + const destination = join(root, 'src') + + await provider.downloadFolder!('/remote/src', destination) + + expect(createSftp).toHaveBeenCalledTimes(1) + expect(sftp.fastGet.mock.calls.map(([source, local]) => [source, local])).toEqual([ + ['/remote/src/index.ts', join(destination, 'index.ts')], + ['/remote/src/lib/a.ts', join(destination, 'lib', 'a.ts')] + ]) + await expect(stat(join(destination, 'lib'))).resolves.toMatchObject({}) + expect(sftp.end).toHaveBeenCalledTimes(1) + expect(mux.request).not.toHaveBeenCalled() + }) + + it('rejects directory symlinks without following them', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-ssh-folder-download-')) + localDownloadRoots.push(root) + const sftp = { + stat: vi.fn( + (remotePath: string, callback: (err: Error | undefined, value: unknown) => void) => + callback(undefined, sftpStats(remotePath === '/remote/src' ? 'directory' : 'directory')) + ), + readdir: vi.fn((_path: string, callback: (err: Error | undefined, value: unknown) => void) => + callback(undefined, [sftpEntry('linked-dir', 'symlink')]) + ), + fastGet: vi.fn(), + end: vi.fn() + } + provider = new SshFilesystemProvider('conn-1', mux as never, async () => sftp as never) + + await expect(provider.downloadFolder!('/remote/src', join(root, 'src'))).rejects.toThrow( + "Cannot download symbolic link 'linked-dir'" + ) + + expect(sftp.fastGet).not.toHaveBeenCalled() + expect(sftp.end).toHaveBeenCalledTimes(1) + }) + + it('rejects remote names that sanitize to the same local filename', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-ssh-folder-download-')) + localDownloadRoots.push(root) + const sftp = { + stat: vi.fn((_path: string, callback: (err: Error | undefined, value: unknown) => void) => + callback(undefined, sftpStats('directory')) + ), + readdir: vi.fn((_path: string, callback: (err: Error | undefined, value: unknown) => void) => + callback(undefined, [sftpEntry('a:b.txt', 'file'), sftpEntry('a?b.txt', 'file')]) + ), + fastGet: vi.fn(), + end: vi.fn() + } + provider = new SshFilesystemProvider('conn-1', mux as never, async () => sftp as never) + + await expect(provider.downloadFolder!('/remote/src', join(root, 'src'))).rejects.toThrow( + "Remote entries map to the same local name 'a_b.txt'" + ) + + expect(sftp.fastGet).not.toHaveBeenCalled() + expect(sftp.end).toHaveBeenCalledTimes(1) + }) + + it('cancels a pending SFTP session open', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-ssh-folder-download-')) + localDownloadRoots.push(root) + const createSftp = vi.fn( + async (options?: { signal?: AbortSignal }) => + new Promise((_resolve, reject) => { + options?.signal?.addEventListener('abort', () => reject(new Error('open canceled')), { + once: true + }) + }) + ) + provider = new SshFilesystemProvider('conn-1', mux as never, createSftp) + const controller = new AbortController() + + const result = provider.downloadFolder!('/remote/src', join(root, 'src'), { + signal: controller.signal + }) + await vi.waitFor(() => expect(createSftp).toHaveBeenCalledTimes(1)) + controller.abort(new Error('renderer closed')) + + await expect(result).rejects.toThrow('open canceled') + expect(createSftp).toHaveBeenCalledWith({ signal: controller.signal }) + expect(mux.request).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/providers/ssh-filesystem-provider-sftp.ts b/src/main/providers/ssh-filesystem-provider-sftp.ts index 31c003303..a3b1b840b 100644 --- a/src/main/providers/ssh-filesystem-provider-sftp.ts +++ b/src/main/providers/ssh-filesystem-provider-sftp.ts @@ -1,6 +1,60 @@ -import type { SFTPWrapper, Stats } from 'ssh2' +import type { FileEntryWithStats, SFTPWrapper, Stats } from 'ssh2' import type { FileStat } from './types' +const ABORTED_SFTP_OPERATION_GRACE_MS = 5_000 + +function abortReason(signal: AbortSignal): Error { + return signal.reason instanceof Error ? signal.reason : new Error('Download canceled') +} + +function waitForSftpCallback( + register: (callback: (err?: Error | null, value?: T) => void) => void, + options?: { signal?: AbortSignal } +): Promise { + return new Promise((resolve, reject) => { + const signal = options?.signal + if (signal?.aborted) { + reject(abortReason(signal)) + return + } + + let settled = false + let abortTimer: ReturnType | undefined + const cleanup = (): void => { + clearTimeout(abortTimer) + signal?.removeEventListener('abort', onAbort) + } + const settle = (error?: Error | null, value?: T): void => { + if (settled) { + return + } + settled = true + cleanup() + if (signal?.aborted) { + reject(abortReason(signal)) + } else if (error) { + reject(error) + } else { + resolve(value as T) + } + } + const onAbort = (): void => { + if (!signal || settled) { + return + } + // Why: the folder owner closes SFTP on abort; wait for its callback so + // Windows local handles quiesce before the temporary tree is removed. + abortTimer = setTimeout(() => settle(abortReason(signal)), ABORTED_SFTP_OPERATION_GRACE_MS) + } + signal?.addEventListener('abort', onAbort, { once: true }) + try { + register((error, value) => settle(error, value)) + } catch (error) { + settle(error instanceof Error ? error : new Error(String(error))) + } + }) +} + export function fileStatFromSftpStats(stats: Stats): FileStat { let type: FileStat['type'] = 'file' if (stats.isDirectory()) { @@ -32,15 +86,30 @@ export function lstatViaSftp(sftp: SFTPWrapper, filePath: string): Promise { - return new Promise((resolve, reject) => { - sftp.fastGet(sourcePath, destinationPath, (err) => { - if (err) { - reject(err) - return - } - resolve() - }) - }) + return waitForSftpCallback( + (callback) => sftp.fastGet(sourcePath, destinationPath, callback), + options + ) +} + +export function readDirViaSftp( + sftp: SFTPWrapper, + dirPath: string, + options?: { signal?: AbortSignal } +): Promise { + return waitForSftpCallback( + (callback) => sftp.readdir(dirPath, callback), + options + ) +} + +export function statViaSftp( + sftp: SFTPWrapper, + filePath: string, + options?: { signal?: AbortSignal } +): Promise { + return waitForSftpCallback((callback) => sftp.stat(filePath, callback), options) } diff --git a/src/main/providers/ssh-filesystem-provider.test.ts b/src/main/providers/ssh-filesystem-provider.test.ts index 55659b18c..a7061616c 100644 --- a/src/main/providers/ssh-filesystem-provider.test.ts +++ b/src/main/providers/ssh-filesystem-provider.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, vi, beforeEach } from 'vitest' +import { beforeEach, describe, expect, it, vi } from 'vitest' import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' diff --git a/src/main/providers/ssh-filesystem-provider.ts b/src/main/providers/ssh-filesystem-provider.ts index b17b3c4f7..74f31b8e6 100644 --- a/src/main/providers/ssh-filesystem-provider.ts +++ b/src/main/providers/ssh-filesystem-provider.ts @@ -1,12 +1,13 @@ import type { SshChannelMultiplexer } from '../ssh/ssh-channel-multiplexer' import { isMethodNotFoundError, readFileViaStream } from '../ssh/ssh-filesystem-stream-reader' import { uploadBuffer } from '../ssh/sftp-upload' -import { fastGetViaSftp, lstatViaSftp } from './ssh-filesystem-provider-sftp' +import { lstatViaSftp } from './ssh-filesystem-provider-sftp' import { - openSshFileUploadSession, - type SftpFactory, - type SshRawTransferOptions -} from './ssh-filesystem-file-upload' + downloadFileViaSftp, + downloadFolderViaSftp, + type SftpFactory +} from './ssh-filesystem-download' +import { openSshFileUploadSession, type SshRawTransferOptions } from './ssh-filesystem-file-upload' import { closeSshFilesystemWatch, registerSshFilesystemWatch, @@ -23,6 +24,7 @@ import type { import type { DirEntry, FsChangeEvent, SearchOptions, SearchResult } from '../../shared/types' import { routeSshFilesystemWatchNotification } from './ssh-filesystem-watch-notifications' import type { WorkspaceSpaceDirectoryScanResult } from '../../shared/workspace-space-types' +import { isWindowsRemoteHost, type RemoteHostPlatform } from '../ssh/ssh-remote-platform' const WORKSPACE_SPACE_SCAN_TIMEOUT_MS = 130_000 export class SshFilesystemProvider implements IFilesystemProvider { @@ -33,16 +35,30 @@ export class SshFilesystemProvider implements IFilesystemProvider { private tempDirPromise: Promise | null = null private disposed = false private loggedStreamFallback = false + readonly downloadFolder?: IFilesystemProvider['downloadFolder'] constructor( connectionId: string, mux: SshChannelMultiplexer, private readonly createSftp?: SftpFactory, - private readonly rawTransfer?: SshRawTransferOptions + private readonly rawTransfer?: SshRawTransferOptions, + hostPlatform?: RemoteHostPlatform ) { this.connectionId = connectionId this.mux = mux + if (createSftp) { + // Why: system SSH has raw single-file transfer but no ssh2 SFTP channel; + // omitting this method makes folder capability truthful at the provider boundary. + // windowsRemotePaths is provider-owned (from host platform), not a caller option. + const windowsRemotePaths = hostPlatform ? isWindowsRemoteHost(hostPlatform) : undefined + this.downloadFolder = (sourcePath, destinationPath, options) => + downloadFolderViaSftp(createSftp, sourcePath, destinationPath, { + ...options, + windowsRemotePaths + }) + } + this.unsubscribeNotifications = mux.onNotification((method, params) => routeSshFilesystemWatchNotification(this.watchListeners, method, params) ) @@ -115,19 +131,12 @@ export class SshFilesystemProvider implements IFilesystemProvider { } async downloadFile(sourcePath: string, destinationPath: string): Promise { + // Why: system SSH targets cannot open an ssh2-owned SFTP channel. if (this.rawTransfer?.downloadFile) { await this.rawTransfer.downloadFile(sourcePath, destinationPath) return } - if (!this.createSftp) { - throw new Error('Remote file download is unavailable. Reconnect the SSH target and retry.') - } - const sftp = await this.createSftp() - try { - await fastGetViaSftp(sftp, sourcePath, destinationPath) - } finally { - sftp.end() - } + await downloadFileViaSftp(this.createSftp, sourcePath, destinationPath) } async openFileUploadSession(): Promise { diff --git a/src/main/providers/types.ts b/src/main/providers/types.ts index d08577f98..2e98dbca2 100644 --- a/src/main/providers/types.ts +++ b/src/main/providers/types.ts @@ -266,6 +266,7 @@ export type IFilesystemProvider = { options: TerminalArtifactAccessOptions ): Promise downloadFile?(sourcePath: string, destinationPath: string): Promise + downloadFolder?: (src: string, dest: string, options?: { signal?: AbortSignal }) => Promise openFileUploadSession?(): Promise getTempDir?(): Promise writeFile(filePath: string, content: string): Promise diff --git a/src/main/runtime/orca-runtime-terminal-cwd.test.ts b/src/main/runtime/orca-runtime-terminal-cwd.test.ts index f8868848e..1ff62b33d 100644 --- a/src/main/runtime/orca-runtime-terminal-cwd.test.ts +++ b/src/main/runtime/orca-runtime-terminal-cwd.test.ts @@ -109,9 +109,8 @@ describe('OrcaRuntimeService terminal startup cwd', () => { it('materializes restored headless mobile tabs in the persisted startup cwd', async () => { const store = { - // wt-1 is a worktree id, not a registered repo, so getRepo returns null; - // the selector validator calls it to reject repo ids passed as worktree ids. - getRepo: () => null, + // wt-1 is a worktree id, not a registered repo, so getRepo returns undefined. + getRepo: () => undefined, getWorkspaceSession: () => ({ activeRepoId: null, activeWorktreeId: 'wt-1', diff --git a/src/main/ssh/ssh-connection.test.ts b/src/main/ssh/ssh-connection.test.ts index 3274d3f5d..37bffc0d6 100644 --- a/src/main/ssh/ssh-connection.test.ts +++ b/src/main/ssh/ssh-connection.test.ts @@ -305,6 +305,7 @@ describe('SshConnection', () => { await conn.connect() expect(conn.getState().status).toBe('connected') + expect(conn.getState().supportsFolderDownload).toBe(true) expect(callbacks.onStateChange).toHaveBeenCalledWith( 'target-1', expect.objectContaining({ status: 'connected' }) @@ -972,6 +973,55 @@ describe('SshConnection', () => { } }) + it('cancels a pending SFTP channel open and ends the late channel', async () => { + const conn = new SshConnection(createTarget(), createCallbacks()) + await conn.connect() + sftpBehavior = 'pending' + const controller = new AbortController() + const lateSftp = { end: vi.fn() } + + const outcomePromise = conn + .sftp({ signal: controller.signal }) + .then(() => 'opened') + .catch((error: Error) => error.name) + + await Promise.resolve() + controller.abort() + pendingSftpCallback?.(undefined, lateSftp) + + await expect(outcomePromise).resolves.toBe('AbortError') + expect(lateSftp.end).toHaveBeenCalledTimes(1) + }) + + it('removes the late SFTP close listener when the bounded grace expires', async () => { + const conn = new SshConnection(createTarget(), createCallbacks()) + await conn.connect() + sftpBehavior = 'pending' + const controller = new AbortController() + const lateSftp = Object.assign(new EventEmitter(), { end: vi.fn() }) + + vi.useFakeTimers() + try { + const outcomePromise = conn + .sftp({ signal: controller.signal }) + .then(() => 'opened') + .catch((error: Error) => error.name) + + await Promise.resolve() + controller.abort() + pendingSftpCallback?.(undefined, lateSftp) + expect(lateSftp.listenerCount('close')).toBe(1) + + await vi.advanceTimersByTimeAsync(5_000) + + await expect(outcomePromise).resolves.toBe('AbortError') + expect(lateSftp.listenerCount('close')).toBe(0) + expect(lateSftp.end).toHaveBeenCalledTimes(1) + } finally { + vi.useRealTimers() + } + }) + it('uses system SSH transport when ProxyUseFdpass is resolved by OpenSSH', async () => { vi.mocked(resolveWithSshG).mockResolvedValueOnce(createResolvedConfig()) const conn = new SshConnection(createTarget({ configHost: 'fdpass-host' }), createCallbacks()) @@ -980,6 +1030,7 @@ describe('SshConnection', () => { expect(conn.getState().status).toBe('connected') expect(conn.usesSystemSshTransport()).toBe(true) + expect(conn.getState().supportsFolderDownload).toBe(false) expect(clientInstances).toHaveLength(0) expect(spawnSystemSshCommandMock).toHaveBeenCalledWith( expect.objectContaining({ configHost: 'fdpass-host' }), diff --git a/src/main/ssh/ssh-connection.ts b/src/main/ssh/ssh-connection.ts index b8bd0cbd5..d38b0ab35 100644 --- a/src/main/ssh/ssh-connection.ts +++ b/src/main/ssh/ssh-connection.ts @@ -96,7 +96,8 @@ export class SshConnection { targetId: target.id, status: 'disconnected', error: null, - reconnectAttempt: 0 + reconnectAttempt: 0, + supportsFolderDownload: false } } @@ -168,7 +169,12 @@ export class SshConnection { ) } - async sftp(signal?: AbortSignal): Promise { + async sftp(options?: AbortSignal | { signal?: AbortSignal }): Promise { + // Why: relay transfers pass a signal directly, while filesystem factories use an options object. + const signal = options && 'aborted' in options ? options : options?.signal + if (signal?.aborted) { + throw createSshOperationAbortError() + } if (this.useSystemSshTransport) { throw new Error('SFTP is not available when using system SSH transport') } @@ -1362,7 +1368,12 @@ export class SshConnection { } private setState(status: SshConnectionStatus, error?: string): void { - this.state = { ...this.state, status, error: error ?? null } + this.state = { + ...this.state, + status, + error: error ?? null, + supportsFolderDownload: status === 'connected' && !this.useSystemSshTransport + } this.callbacks.onStateChange(this.target.id, { ...this.state }) } } diff --git a/src/main/ssh/ssh-relay-session-terminal-error.test.ts b/src/main/ssh/ssh-relay-session-terminal-error.test.ts index 0fab723fb..56d12ec9c 100644 --- a/src/main/ssh/ssh-relay-session-terminal-error.test.ts +++ b/src/main/ssh/ssh-relay-session-terminal-error.test.ts @@ -7,6 +7,10 @@ import type { SshPortForwardManager } from './ssh-port-forward' import { RelayVersionMismatchError } from './ssh-relay-version-mismatch-error' import type { BrowserWindow } from 'electron' +const { filesystemProviderConstructorMock } = vi.hoisted(() => ({ + filesystemProviderConstructorMock: vi.fn() +})) + vi.mock('./ssh-relay-deploy', () => ({ deployAndLaunchRelay: vi.fn() })) @@ -39,6 +43,9 @@ vi.mock('../providers/ssh-pty-provider', () => ({ vi.mock('../providers/ssh-filesystem-provider', () => ({ SshFilesystemProvider: class MockSshFilesystemProvider { + constructor(...args: unknown[]) { + filesystemProviderConstructorMock(...args) + } dispose = vi.fn() } })) @@ -114,9 +121,28 @@ function mockDeploySuccess(): void { describe('SshRelaySession terminal relay error (RelayVersionMismatchError)', () => { beforeEach(() => { vi.clearAllMocks() + filesystemProviderConstructorMock.mockReset() mockDeploySuccess() }) + it('omits the SFTP folder factory while retaining raw transfer on system SSH', async () => { + const { mockStore, mockPortForward, getMainWindow } = createMockDeps() + const mockConn = { + usesSystemSshTransport: vi.fn(() => true) + } as unknown as SshConnection + const session = new SshRelaySession('target-1', getMainWindow, mockStore, mockPortForward) + + await session.establish(mockConn) + + expect(filesystemProviderConstructorMock).toHaveBeenCalledWith( + 'target-1', + expect.anything(), + undefined, + expect.objectContaining({ downloadFile: expect.any(Function) }), + undefined + ) + }) + it('fires onTerminalRelayError on initial establish() and rethrows', async () => { const { mockConn, mockStore, mockPortForward, getMainWindow } = createMockDeps() const session = new SshRelaySession('target-1', getMainWindow, mockStore, mockPortForward) diff --git a/src/main/ssh/ssh-relay-session.ts b/src/main/ssh/ssh-relay-session.ts index df274457f..11f7f5f93 100644 --- a/src/main/ssh/ssh-relay-session.ts +++ b/src/main/ssh/ssh-relay-session.ts @@ -675,26 +675,35 @@ export class SshRelaySession { const ptyProvider = new SshPtyProvider(this.targetId, mux, this.remoteCliBridgeEnv ?? undefined) registerSshPtyProvider(this.targetId, ptyProvider) + const connection = this.requireReadyConnection() + const createSftp = + connection.usesSystemSshTransport?.() === true + ? undefined + : (options?: { signal?: AbortSignal }) => this.requireReadyConnection().sftp(options) + // Why: getHostPlatform() falls back to this.hostPlatform when the full + // remote CLI bridge env is incomplete, so path rules still match the host. + const hostPlatform = this.getHostPlatform() ?? undefined const fsProvider = new SshFilesystemProvider( this.targetId, mux, - () => this.requireReadyConnection().sftp(), + createSftp, { downloadFile: (sourcePath, destinationPath) => this.requireReadyConnection().downloadFile(sourcePath, destinationPath, { - hostPlatform: this.remoteCliBridgeEnv?.hostPlatform + hostPlatform }), openFileUploadSession: () => this.requireReadyConnection().openFileUploadSession({ - hostPlatform: this.remoteCliBridgeEnv?.hostPlatform + hostPlatform }), writeBuffer: (remotePath, contents, options) => this.requireReadyConnection().writeBuffer(remotePath, contents, { - hostPlatform: this.remoteCliBridgeEnv?.hostPlatform, + hostPlatform, append: options.append, exclusive: options.exclusive }) - } + }, + hostPlatform ) registerSshFilesystemProvider(this.targetId, fsProvider) diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index f77783f9c..8fbe1a6ac 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -2478,6 +2478,10 @@ export type PreloadApi = { filePath: string connectionId: string }) => Promise<{ canceled: true } | { canceled: false; destinationPath: string }> + downloadFolder: (args: { + dirPath: string + connectionId: string + }) => Promise<{ canceled: true } | { canceled: false; destinationPath: string }> saveDownloadedFile: (args: { suggestedName: string content: string diff --git a/src/preload/index.ts b/src/preload/index.ts index d1c1b4cb7..59097c34f 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -2834,6 +2834,11 @@ const api = { connectionId: string }): Promise<{ canceled: true } | { canceled: false; destinationPath: string }> => ipcRenderer.invoke('fs:downloadFile', args), + downloadFolder: (args: { + dirPath: string + connectionId: string + }): Promise<{ canceled: true } | { canceled: false; destinationPath: string }> => + ipcRenderer.invoke('fs:downloadFolder', args), saveDownloadedFile: (args: { suggestedName: string content: string diff --git a/src/renderer/src/components/right-sidebar/FileExplorer.test.tsx b/src/renderer/src/components/right-sidebar/FileExplorer.test.tsx index f8067c7df..1688b0721 100644 --- a/src/renderer/src/components/right-sidebar/FileExplorer.test.tsx +++ b/src/renderer/src/components/right-sidebar/FileExplorer.test.tsx @@ -587,7 +587,7 @@ describe('FileExplorerRow collapse folder action', () => { expect(shouldShowViewFileAction(directoryNode)).toBe(false) }) - it('shows remote download only for desktop SSH or Remote Host file-like rows', () => { + it('shows remote download only for desktop SSH rows and file-like Remote Host rows', () => { const runtimeContext = { settings: { activeRuntimeEnvironmentId: 'runtime-1' }, worktreeId: 'wt-1', @@ -598,7 +598,12 @@ describe('FileExplorerRow collapse folder action', () => { expect(shouldShowRemoteDownloadAction({ ...fileNode, isSymlink: true }, 'ssh-1')).toBe(true) expect(shouldShowRemoteDownloadAction(fileNode, null, runtimeContext)).toBe(true) expect(shouldShowRemoteDownloadAction(fileNode, null)).toBe(false) + // Why: directory download defaults fail-closed until the connection advertises + // supportsFolderDownload (SFTP); system-SSH and unknown capability stay hidden. expect(shouldShowRemoteDownloadAction(directoryNode, 'ssh-1')).toBe(false) + expect(shouldShowRemoteDownloadAction(directoryNode, 'ssh-1', null, true)).toBe(true) + expect(shouldShowRemoteDownloadAction(directoryNode, 'ssh-1', null, false)).toBe(false) + expect(shouldShowRemoteDownloadAction(fileNode, 'ssh-1', null, false)).toBe(true) expect(shouldShowRemoteDownloadAction(directoryNode, null, runtimeContext)).toBe(false) ;(globalThis as { __ORCA_WEB_CLIENT__?: boolean }).__ORCA_WEB_CLIENT__ = true @@ -708,6 +713,38 @@ describe('FileExplorerRow collapse folder action', () => { expect(toastErrorMock).not.toHaveBeenCalled() }) + it('calls the preload folder download API for SSH directory rows', async () => { + const downloadFolder = vi.fn().mockResolvedValue({ + canceled: false, + destinationPath: '/downloads/src' + }) + const openPath = vi.fn().mockResolvedValue(undefined) + ;( + globalThis as unknown as { + window: { + api: { + fs: { downloadFolder: typeof downloadFolder } + shell: { openPath: typeof openPath } + } + } + } + ).window = { api: { fs: { downloadFolder }, shell: { openPath } } } + + await downloadRemoteFile(directoryNode, 'ssh-1') + + expect(downloadFolder).toHaveBeenCalledWith({ + dirPath: '/repo/src', + connectionId: 'ssh-1' + }) + expect(toastSuccessMock).toHaveBeenCalledWith("Downloaded folder 'src'", { + action: { + label: 'Open', + onClick: expect.any(Function) + } + }) + expect(toastErrorMock).not.toHaveBeenCalled() + }) + it('downloads Remote Host rows through the runtime download path', async () => { const runtimeContext = { settings: { activeRuntimeEnvironmentId: 'runtime-1' }, diff --git a/src/renderer/src/components/right-sidebar/FileExplorer.tsx b/src/renderer/src/components/right-sidebar/FileExplorer.tsx index 7f2c43835..d919cb1b6 100644 --- a/src/renderer/src/components/right-sidebar/FileExplorer.tsx +++ b/src/renderer/src/components/right-sidebar/FileExplorer.tsx @@ -80,6 +80,12 @@ function FileExplorerFiles(): React.JSX.Element { const activeWorktreeId = useAppStore((s) => s.activeWorktreeId) const activeWorktree = useActiveWorktree() const activeRepo = useRepoById(activeWorktree?.repoId ?? null) + const supportsFolderDownload = useAppStore((s) => { + const connectionId = activeRepo?.connectionId + return connectionId + ? s.sshConnectionStates.get(connectionId)?.supportsFolderDownload === true + : false + }) const activeRuntimeEnvironmentId = useAppStore((s) => getRuntimeEnvironmentIdForWorktree(s, activeWorktreeId) ) @@ -746,6 +752,7 @@ function FileExplorerFiles(): React.JSX.Element { deleteShortcutLabel={deleteShortcutLabel} connectionId={activeRepo?.connectionId ?? null} runtimeDownloadContext={runtimeDownloadContext} + supportsFolderDownload={supportsFolderDownload} onClick={handleRowClick} onDoubleClick={handleDoubleClick} onViewFile={handleClick} diff --git a/src/renderer/src/components/right-sidebar/FileExplorerRow.tsx b/src/renderer/src/components/right-sidebar/FileExplorerRow.tsx index 6c4ace896..4d260e09a 100644 --- a/src/renderer/src/components/right-sidebar/FileExplorerRow.tsx +++ b/src/renderer/src/components/right-sidebar/FileExplorerRow.tsx @@ -273,6 +273,7 @@ type FileExplorerRowProps = { deleteShortcutLabel: string connectionId?: string | null runtimeDownloadContext?: RuntimeFileOperationArgs | null + supportsFolderDownload?: boolean canCollapseFolderSubtree: boolean targetDir: string targetDepth: number @@ -318,12 +319,18 @@ export function shouldShowViewFileAction(node: TreeNode): boolean { export function shouldShowRemoteDownloadAction( node: TreeNode, connectionId?: string | null, - runtimeDownloadContext?: RuntimeFileOperationArgs | null + runtimeDownloadContext?: RuntimeFileOperationArgs | null, + // Why: fail closed — only show folder download when the connection explicitly + // advertises SFTP recursive transfer (system-SSH and unknown states stay off). + supportsFolderDownload = false ): boolean { - // Why: Desktop-only because download depends on Electron's native save dialog. + // Why: Desktop-only because download depends on Electron's native save/folder dialogs; + // runtime and system-SSH folders have no recursive transfer contract. + const hasDownloadCapability = node.isDirectory + ? Boolean(connectionId && supportsFolderDownload) + : Boolean(connectionId || runtimeDownloadContext) return ( - !node.isDirectory && - Boolean(connectionId || runtimeDownloadContext) && + hasDownloadCapability && (globalThis as { __ORCA_WEB_CLIENT__?: boolean }).__ORCA_WEB_CLIENT__ !== true ) } @@ -349,21 +356,32 @@ export async function downloadRemoteFile( try { const result = typeof connectionIdOrRuntimeContext === 'string' - ? await window.api.fs.downloadFile({ - filePath: node.path, - connectionId: connectionIdOrRuntimeContext - }) + ? node.isDirectory + ? await window.api.fs.downloadFolder({ + dirPath: node.path, + connectionId: connectionIdOrRuntimeContext + }) + : await window.api.fs.downloadFile({ + filePath: node.path, + connectionId: connectionIdOrRuntimeContext + }) : await downloadRuntimeFile(connectionIdOrRuntimeContext, node.path, node.name) // Why: Suppress toasts when the user cancels the native save dialog per design. if (result.canceled) { return } toast.success( - translate( - 'auto.components.right.sidebar.FileExplorerRow.bce4d4e44f', - "Downloaded '{{value0}}'", - { value0: node.name } - ), + node.isDirectory + ? translate( + 'auto.components.right.sidebar.FileExplorerRow.a4029c996b', + "Downloaded folder '{{value0}}'", + { value0: node.name } + ) + : translate( + 'auto.components.right.sidebar.FileExplorerRow.bce4d4e44f', + "Downloaded '{{value0}}'", + { value0: node.name } + ), { action: { label: translate('auto.components.right.sidebar.FileExplorerRow.1a3df04ae1', 'Open'), @@ -377,11 +395,17 @@ export async function downloadRemoteFile( toast.error( extractIpcErrorMessage( error, - translate( - 'auto.components.right.sidebar.FileExplorerRow.b3e288bf41', - "Failed to download '{{value0}}'.", - { value0: node.name } - ) + node.isDirectory + ? translate( + 'auto.components.right.sidebar.FileExplorerRow.f729bcd97d', + "Failed to download folder '{{value0}}'.", + { value0: node.name } + ) + : translate( + 'auto.components.right.sidebar.FileExplorerRow.b3e288bf41', + "Failed to download '{{value0}}'.", + { value0: node.name } + ) ) ) } @@ -420,6 +444,7 @@ export function FileExplorerRow({ deleteShortcutLabel, connectionId, runtimeDownloadContext, + supportsFolderDownload = false, canCollapseFolderSubtree, targetDir, targetDepth, @@ -455,7 +480,8 @@ export function FileExplorerRow({ const showRemoteDownloadAction = shouldShowRemoteDownloadAction( node, connectionId, - runtimeDownloadContext + runtimeDownloadContext, + supportsFolderDownload ) const showCopyFileAction = shouldShowCopyFileAction(node, connectionId, selectionSize) const { setRowDragNode, handleDragOver, handleDragEnter, handleDragLeave, handleDrop } = @@ -754,7 +780,12 @@ export function FileExplorerRow({ {showRemoteDownloadAction && ( - {translate('auto.components.right.sidebar.FileExplorerRow.c2112579f6', 'Download')} + {node.isDirectory + ? translate( + 'auto.components.right.sidebar.FileExplorerRow.7ac885bd2f', + 'Download Folder' + ) + : translate('auto.components.right.sidebar.FileExplorerRow.c2112579f6', 'Download')} )} {canCollapseFolderSubtree && shouldShowCollapseFolderAction(node, isExpanded) && ( diff --git a/src/renderer/src/components/right-sidebar/FileExplorerVirtualRows.tsx b/src/renderer/src/components/right-sidebar/FileExplorerVirtualRows.tsx index 5c64a5a04..67a52bb7c 100644 --- a/src/renderer/src/components/right-sidebar/FileExplorerVirtualRows.tsx +++ b/src/renderer/src/components/right-sidebar/FileExplorerVirtualRows.tsx @@ -28,6 +28,7 @@ type FileExplorerVirtualRowsProps = { deleteShortcutLabel: string connectionId?: string | null runtimeDownloadContext?: RuntimeFileOperationArgs | null + supportsFolderDownload?: boolean onClick: (node: TreeNode, event: React.MouseEvent) => void onDoubleClick: (node: TreeNode) => void onViewFile: (node: TreeNode) => void @@ -73,6 +74,7 @@ export function FileExplorerVirtualRows(props: FileExplorerVirtualRowsProps): Re deleteShortcutLabel, connectionId, runtimeDownloadContext, + supportsFolderDownload = false, onClick, onDoubleClick, onViewFile, @@ -176,6 +178,7 @@ export function FileExplorerVirtualRows(props: FileExplorerVirtualRowsProps): Re deleteShortcutLabel={deleteShortcutLabel} connectionId={connectionId} runtimeDownloadContext={runtimeDownloadContext} + supportsFolderDownload={supportsFolderDownload} canCollapseFolderSubtree={canCollapseFolderSubtree} targetDir={n.isDirectory ? n.path : dirname(n.path)} targetDepth={n.isDirectory ? n.depth + 1 : n.depth} diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 0265e2770..534bcdd32 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -9338,14 +9338,17 @@ "d6a25618aa": "Collapse Folder", "d87a4c42e1": "Open Markdown Preview", "c2112579f6": "Download", + "7ac885bd2f": "Download Folder", "dd112c81d2": "Open in Orca Browser", "1bb9be455c": "Add as Project...", "0fec99bfd7": "Duplicate", "f61af83316": "New Folder", "37c875d827": "New File", "b3e288bf41": "Failed to download '{{value0}}'.", + "f729bcd97d": "Failed to download folder '{{value0}}'.", "1a3df04ae1": "Open", "bce4d4e44f": "Downloaded '{{value0}}'", + "a4029c996b": "Downloaded folder '{{value0}}'", "e26010014a": "Ignored by .gitignore", "a06551beee": "Enter", "128a99ed5e": "Unassigned", diff --git a/src/renderer/src/i18n/locales/es.json b/src/renderer/src/i18n/locales/es.json index 414ad493f..3a5311cb7 100644 --- a/src/renderer/src/i18n/locales/es.json +++ b/src/renderer/src/i18n/locales/es.json @@ -9315,14 +9315,17 @@ "d6a25618aa": "Contraer carpeta", "d87a4c42e1": "Abrir vista previa de Markdown", "c2112579f6": "Descargar", + "7ac885bd2f": "Descargar carpeta", "dd112c81d2": "Abrir en Orca Browser", "1bb9be455c": "Agregar como proyecto…", "0fec99bfd7": "Duplicar", "f61af83316": "Nueva carpeta", "37c875d827": "Nuevo archivo", "b3e288bf41": "No se pudo descargar '{{value0}}'.", + "f729bcd97d": "No se pudo descargar la carpeta '{{value0}}'.", "1a3df04ae1": "Abrir", "bce4d4e44f": "Se descargó '{{value0}}'", + "a4029c996b": "Se descargó la carpeta '{{value0}}'", "e26010014a": "Ignorado por .gitignore", "a06551beee": "Intro", "128a99ed5e": "No asignado", diff --git a/src/renderer/src/i18n/locales/ja.json b/src/renderer/src/i18n/locales/ja.json index ed94565f6..9cd63fa6b 100644 --- a/src/renderer/src/i18n/locales/ja.json +++ b/src/renderer/src/i18n/locales/ja.json @@ -9315,14 +9315,17 @@ "d6a25618aa": "フォルダを折りたたむ", "d87a4c42e1": "Markdown プレビューを開く", "c2112579f6": "ダウンロード", + "7ac885bd2f": "フォルダーをダウンロード", "dd112c81d2": "Orca ブラウザで開く", "1bb9be455c": "プロジェクトとして追加...", "0fec99bfd7": "重複", "f61af83316": "新規フォルダー", "37c875d827": "新規ファイル", "b3e288bf41": "「{{value0}}」のダウンロードに失敗しました。", + "f729bcd97d": "フォルダー「{{value0}}」のダウンロードに失敗しました。", "1a3df04ae1": "オープン", "bce4d4e44f": "「{{value0}}」をダウンロードしました", + "a4029c996b": "フォルダー「{{value0}}」をダウンロードしました", "e26010014a": ".gitignore によって無視される", "a06551beee": "入力", "128a99ed5e": "未割り当て", diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index d3823c0a6..e8d8ba493 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -9315,14 +9315,17 @@ "d6a25618aa": "폴더 축소", "d87a4c42e1": "Markdown 미리보기 열기", "c2112579f6": "다운로드", + "7ac885bd2f": "폴더 다운로드", "dd112c81d2": "Orca 브라우저에서 열기", "1bb9be455c": "프로젝트로 추가...", "0fec99bfd7": "복제", "f61af83316": "새 폴더", "37c875d827": "새 파일", "b3e288bf41": "'{{value0}}'을(를) 다운로드하지 못했습니다.", + "f729bcd97d": "'{{value0}}' 폴더를 다운로드하지 못했습니다.", "1a3df04ae1": "열기", "bce4d4e44f": "'{{value0}}'을(를) 다운로드했습니다.", + "a4029c996b": "'{{value0}}' 폴더를 다운로드했습니다.", "e26010014a": ".gitignore에 의해 무시됨", "a06551beee": "입력", "128a99ed5e": "할당되지 않음", diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index 8ca2fdb7e..2ec328ba2 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -9315,14 +9315,17 @@ "d6a25618aa": "折叠文件夹", "d87a4c42e1": "打开 Markdown 预览", "c2112579f6": "下载", + "7ac885bd2f": "下载文件夹", "dd112c81d2": "在 Orca 浏览器中打开", "1bb9be455c": "添加为项目...", "0fec99bfd7": "复制", "f61af83316": "新建文件夹", "37c875d827": "新文件", "b3e288bf41": "无法下载“{{value0}}”。", + "f729bcd97d": "无法下载文件夹“{{value0}}”。", "1a3df04ae1": "开放", "bce4d4e44f": "下载“{{value0}}”", + "a4029c996b": "已下载文件夹“{{value0}}”", "e26010014a": "被 .gitignore 忽略", "a06551beee": "进入", "128a99ed5e": "未分配", diff --git a/src/renderer/src/store/slices/ssh-target-cleanup.ts b/src/renderer/src/store/slices/ssh-target-cleanup.ts index 704696f68..800e58cdd 100644 --- a/src/renderer/src/store/slices/ssh-target-cleanup.ts +++ b/src/renderer/src/store/slices/ssh-target-cleanup.ts @@ -11,6 +11,7 @@ export function sshConnectionStatesEqual( a?.status === b.status && a?.error === b.error && a?.reconnectAttempt === b.reconnectAttempt && + a?.supportsFolderDownload === b.supportsFolderDownload && a?.remotePlatform === b.remotePlatform ) } diff --git a/src/renderer/src/store/slices/ssh.test.ts b/src/renderer/src/store/slices/ssh.test.ts index ef5ebf677..bd5ca9321 100644 --- a/src/renderer/src/store/slices/ssh.test.ts +++ b/src/renderer/src/store/slices/ssh.test.ts @@ -190,6 +190,30 @@ describe('createSshSlice', () => { expect(store.getState().sshConnectedGeneration).toBe(1) }) + it('publishes a connected-state folder capability change', () => { + const store = createTestStore() + store.getState().setSshConnectionState('ssh-1', { + targetId: 'ssh-1', + status: 'connected', + error: null, + reconnectAttempt: 0, + supportsFolderDownload: false + }) + const previousState = store.getState() + + store.getState().setSshConnectionState('ssh-1', { + targetId: 'ssh-1', + status: 'connected', + error: null, + reconnectAttempt: 0, + supportsFolderDownload: true + }) + + expect(store.getState()).not.toBe(previousState) + expect(store.getState().sshConnectionStates.get('ssh-1')?.supportsFolderDownload).toBe(true) + expect(store.getState().sshConnectedGeneration).toBe(1) + }) + it('does not publish state when cleanup finds no removed SSH target state', () => { const store = createTestStore() const previousState = store.getState() diff --git a/src/renderer/src/web/web-preload-api.test.ts b/src/renderer/src/web/web-preload-api.test.ts index 102b45c67..79e3a846b 100644 --- a/src/renderer/src/web/web-preload-api.test.ts +++ b/src/renderer/src/web/web-preload-api.test.ts @@ -2219,6 +2219,9 @@ describe('web file preload API', () => { await expect( api.fs.downloadFile({ filePath: '/workspace/repo/file.txt', connectionId: 'ssh-1' }) ).rejects.toThrow('Remote file download is unavailable in paired web clients.') + await expect( + api.fs.downloadFolder({ dirPath: '/workspace/repo/src', connectionId: 'ssh-1' }) + ).rejects.toThrow('Remote folder download is unavailable in paired web clients.') }) it('rejects SSH clone requests in paired web clients', async () => { diff --git a/src/renderer/src/web/web-preload-api.ts b/src/renderer/src/web/web-preload-api.ts index f63316a5c..d5f8f2c59 100644 --- a/src/renderer/src/web/web-preload-api.ts +++ b/src/renderer/src/web/web-preload-api.ts @@ -1621,6 +1621,9 @@ function createFileApi(): NonNullable['fs']> { downloadFile: async () => { throw new Error('Remote file download is unavailable in paired web clients.') }, + downloadFolder: async () => { + throw new Error('Remote folder download is unavailable in paired web clients.') + }, saveDownloadedFile: async () => { throw new Error('Remote file download is unavailable in paired web clients.') }, diff --git a/src/shared/ssh-types.ts b/src/shared/ssh-types.ts index 112722b28..8cbcda689 100644 --- a/src/shared/ssh-types.ts +++ b/src/shared/ssh-types.ts @@ -114,6 +114,8 @@ export type SshConnectionState = { error: string | null /** Number of reconnection attempts since last disconnect. */ reconnectAttempt: number + /** Folder downloads require ssh2 SFTP and are unavailable on system SSH. */ + supportsFolderDownload?: boolean /** Remote OS detected by the SSH relay once available. */ remotePlatform?: SshRemotePlatform }