From 86371329bdb4d67f54e7262782e8855e895ab738 Mon Sep 17 00:00:00 2001 From: deanxbox Date: Sun, 18 Oct 2026 19:43:37 +0100 Subject: [PATCH 1/2] feat: improve uploads and add external editor support Pipeline SFTP writes with bounded concurrency to avoid waiting for a round trip after every chunk, and upload up to four files concurrently with one progress dialog showing file and byte counts, themed progress, live transfer speed, and cancellation. Add an Open Externally context-menu action for text files, with guarded UTF-8 decoding, large-file confirmation, and serialized saves. Prevent Enter and Escape in rename and create inputs from triggering the grid's global keyboard shortcuts. Route single- and multi-file drops through the shared upload dialog. --- locales/en.json | 2 + src/backend/content-routes.ts | 82 ++--- src/backend/sftp-pipelined-write.ts | 98 ++++++ src/frontend/FileManager.tsx | 316 +++++++++++------- src/frontend/FileManagerContextMenu.tsx | 11 + src/frontend/FileManagerGrid.tsx | 9 + .../components/UploadProgressToast.tsx | 65 ++++ tests/backend/sftp-pipelined-write.test.ts | 102 ++++++ tests/backend/upload-cancellation.test.ts | 16 +- tests/frontend/FileManagerGrid.test.tsx | 21 ++ 10 files changed, 548 insertions(+), 174 deletions(-) create mode 100644 src/backend/sftp-pipelined-write.ts create mode 100644 src/frontend/components/UploadProgressToast.tsx create mode 100644 tests/backend/sftp-pipelined-write.test.ts diff --git a/locales/en.json b/locales/en.json index ce1008d..1deaf49 100644 --- a/locales/en.json +++ b/locales/en.json @@ -73,6 +73,8 @@ "newFolder": "New Folder", "rename": "Rename", "uploading": "Uploading...", + "externalEditorTextOnly": "Only text files can be opened externally, because saves are written back as text.", + "uploadFilesDone": "{{done}} / {{total}} files", "uploadingFile": "Uploading {{name}}...", "uploadingChunk": "Uploading {{name}}: {{percent}}% (part {{chunk}} of {{total}})", "fileName": "File Name", diff --git a/src/backend/content-routes.ts b/src/backend/content-routes.ts index 16dcbe9..1cc14f7 100644 --- a/src/backend/content-routes.ts +++ b/src/backend/content-routes.ts @@ -1,3 +1,4 @@ +import { pipelineWriteToSftp } from "./sftp-pipelined-write.js"; import { randomUUID } from "node:crypto"; import type { Express, Request, Response } from "express"; import Busboy from "busboy"; @@ -1372,9 +1373,9 @@ export function registerFileContentRoutes( fileStream.resume(); return; } - const writeStream = sftp.createWriteStream(fullPath); + const upload = pipelineWriteToSftp(sftp, fullPath, fileStream); const removePartialFile = () => { - writeStream.destroy(); + upload.abort(); sftp.unlink(fullPath, (err) => { if (err) { ctx.log.error( @@ -1386,60 +1387,31 @@ export function registerFileContentRoutes( }; destroyUpload = removePartialFile; - writeStream.on("error", (err) => { - ctx.log.error("SFTP write stream error during upload:", err); - if (!resolved && !requestAborted) { + upload.done.then( + () => { + if (resolved || requestAborted) return; resolved = true; - res - .status(500) - .json({ error: `Upload failed: ${err.message}` }); - } - }); - - const completeUpload = () => { - if (resolved || requestAborted) return; - resolved = true; - ctx.log.info( - `Streaming file upload completed: ${sessionId} (${fullPath}, duration=${Date.now() - uploadStartTime}ms)`, - ); - res.json({ - message: "File uploaded successfully", - path: fullPath, - toast: { - type: "success", - message: `File uploaded: ${fullPath}`, - }, - }); - }; - - let sourceEnded = false; - fileStream.on("end", () => { - sourceEnded = true; - }); - writeStream.on("finish", completeUpload); - // ssh2's SFTP WriteStream destroys itself inside _final (autoClose), - // and a Writable destroyed before its final callback never emits - // 'finish' on current Node. 'close' fires once the handle has been - // closed after the last write was acknowledged, so it is the - // completion signal that reliably arrives -- but only trust it - // when the whole body was consumed and nothing aborted the upload. - writeStream.on("close", () => { - if (sourceEnded) completeUpload(); - }); - - fileStream.on("error", (err) => { - ctx.log.error("File read stream error during upload:", err); - writeStream.destroy(); - if (!resolved && !requestAborted) { - resolved = true; - res - .status(500) - .json({ error: `Upload stream error: ${err.message}` }); - } - }); - - (fileStream as NodeJS.ReadableStream).pipe( - writeStream as unknown as NodeJS.WritableStream, + ctx.log.info( + `Streaming file upload completed: ${sessionId} (${fullPath}, duration=${Date.now() - uploadStartTime}ms)`, + ); + res.json({ + message: "File uploaded successfully", + path: fullPath, + toast: { + type: "success", + message: `File uploaded: ${fullPath}`, + }, + }); + }, + (err: Error) => { + ctx.log.error("SFTP write error during upload:", err); + if (!resolved && !requestAborted) { + resolved = true; + res + .status(500) + .json({ error: `Upload failed: ${err.message}` }); + } + }, ); }) .catch((err: Error) => { diff --git a/src/backend/sftp-pipelined-write.ts b/src/backend/sftp-pipelined-write.ts new file mode 100644 index 0000000..28cbfda --- /dev/null +++ b/src/backend/sftp-pipelined-write.ts @@ -0,0 +1,98 @@ +type SFTPWrapper = import("ssh2").SFTPWrapper; + +const WRITE_CHUNK = 64 * 1024; +const MAX_IN_FLIGHT = 64; + +/** + * Streams `source` into a remote file with many WRITE requests in flight. + * ssh2's createWriteStream waits for each ack before the next write, which + * caps throughput at one chunk per round trip. + */ +export function pipelineWriteToSftp( + sftp: SFTPWrapper, + path: string, + source: NodeJS.ReadableStream, +): { done: Promise; abort: () => void } { + let abortHook: () => void = () => {}; + + const done = new Promise((resolve, reject) => { + let handle: Buffer | undefined; + let inFlight = 0; + let ended = false; + let settled = false; + let written = 0; + let position = 0; + const queue: Buffer[] = []; + + const closeHandle = (cb: (err?: Error) => void) => { + if (!handle) return cb(); + const h = handle; + handle = undefined; + sftp.close(h, (err) => cb(err ?? undefined)); + }; + const fail = (err: Error) => { + if (settled) return; + settled = true; + queue.length = 0; + source.removeListener("data", onData); + // Drain the rest of the body so the HTTP request can finish cleanly. + source.resume(); + closeHandle(() => reject(err)); + }; + const maybeFinish = () => { + if (settled || !ended || inFlight > 0 || queue.length > 0) return; + settled = true; + closeHandle((err) => (err ? reject(err) : resolve(written))); + }; + // Dispatch queued bytes while fewer than MAX_IN_FLIGHT writes are pending. + const pump = () => { + while ( + !settled && + handle && + queue.length > 0 && + inFlight < MAX_IN_FLIGHT + ) { + const buf = queue[0]; + const len = Math.min(WRITE_CHUNK, buf.length); + const piece = buf.subarray(0, len); + if (len === buf.length) queue.shift(); + else queue[0] = buf.subarray(len); + inFlight++; + sftp.write(handle, piece, 0, len, position, (err) => { + inFlight--; + if (err) return fail(err); + written += len; + pump(); + maybeFinish(); + }); + position += len; + } + if (!settled && !ended && queue.length === 0) source.resume(); + }; + function onData(chunk: Buffer) { + source.pause(); + queue.push(chunk); + pump(); + } + + abortHook = () => fail(new Error("Upload aborted")); + // Attached before open() so an early source error rejects instead of throwing. + source.on("error", fail); + source.once("end", () => { + ended = true; + maybeFinish(); + }); + + sftp.open(path, "w", (err, h) => { + if (err) return fail(err); + if (settled) { + sftp.close(h, () => {}); + return; + } + handle = h; + source.on("data", onData); + }); + }); + + return { done, abort: () => abortHook() }; +} diff --git a/src/frontend/FileManager.tsx b/src/frontend/FileManager.tsx index 5f06706..d8085d0 100644 --- a/src/frontend/FileManager.tsx +++ b/src/frontend/FileManager.tsx @@ -22,6 +22,7 @@ import { } from "./components/WindowManager.tsx"; import { FileWindow } from "./components/FileWindow.tsx"; import { DownloadProgressToast } from "./components/DownloadProgressToast.tsx"; +import { UploadProgressToast } from "./components/UploadProgressToast.tsx"; import { DiffWindow } from "./components/DiffWindow.tsx"; import { useDragToDesktop } from "./hooks/useDragToDesktop"; import { useDragToSystemDesktop } from "./hooks/useDragToSystemDesktop"; @@ -57,6 +58,8 @@ import { copyToClipboard } from "@termix-ssh/plugin-sdk/ui"; import { listSSHFiles, readSSHFile, + writeSSHFile, + downloadSSHFile, resolveSSHPath, uploadSSHFile, createSSHFile, @@ -1189,15 +1192,47 @@ function FileManagerContent({ if (files.length === 0 && emptyDirs.length === 0) return; + await uploadBatch(files, emptyDirs); + } + + async function uploadBatch( + files: { file: File; relativePath: string }[], + emptyDirs: string[], + ) { const controller = new AbortController(); - const cancel = { - label: t("fileManager.cancel"), - onClick: () => controller.abort(), + const totalBytes = files.reduce((n, f) => n + f.file.size, 0); + const toastId = toast.loading(null, { duration: Infinity }); + const sent = new Map(); + let doneFiles = 0; + let lastLoaded = 0; + let lastTime = Date.now(); + let mbPerSec: number | undefined; + + const render = () => { + if (controller.signal.aborted) return; + let loaded = 0; + for (const v of sent.values()) loaded += v; + const now = Date.now(); + if (now - lastTime >= 500) { + mbPerSec = + ((loaded - lastLoaded) / (now - lastTime) / 1024 / 1024) * 1000; + lastLoaded = loaded; + lastTime = now; + } + toast.loading( + controller.abort()} + />, + { id: toastId, duration: Infinity }, + ); }; - const progressToast = toast.loading( - t("fileManager.uploadingFolderFiles", { count: files.length }), - { duration: Infinity, cancel }, - ); + render(); const failed: string[] = []; @@ -1235,35 +1270,46 @@ function FileManagerContent({ ); } - for (const { file, relativePath } of files) { - controller.signal.throwIfAborted(); - const dirPart = relativePath.includes("/") - ? relativePath.substring(0, relativePath.lastIndexOf("/")) - : ""; - const uploadPath = dirPart ? `${base}${dirPart}/` : currentPath; - - try { - await uploadSSHFile( - sshSessionId, - uploadPath, - file.name, - file, - currentHost?.id, - undefined, - undefined, - controller.signal, - ); - } catch (error) { + const ticker = setInterval(render, 500); + try { + await runWithConcurrency(files, 4, async ({ file, relativePath }) => { controller.signal.throwIfAborted(); - failed.push(relativePath); - } + const dirPart = relativePath.includes("/") + ? relativePath.substring(0, relativePath.lastIndexOf("/")) + : ""; + const uploadPath = dirPart ? `${base}${dirPart}/` : base; + + try { + await uploadSSHFile( + sshSessionId, + uploadPath, + file.name, + file, + currentHost?.id, + undefined, + (p) => sent.set(relativePath, p.bytesSent), + controller.signal, + ); + sent.set(relativePath, file.size); + } catch (error) { + controller.signal.throwIfAborted(); + failed.push(relativePath); + } + doneFiles++; + }); + } finally { + clearInterval(ticker); } controller.signal.throwIfAborted(); - toast.dismiss(progressToast); + toast.dismiss(toastId); if (failed.length === 0) { toast.success( - t("fileManager.uploadedFolderFiles", { count: files.length }), + files.length === 1 && emptyDirs.length === 0 + ? t("fileManager.fileUploadedSuccessfully", { + name: files[0].file.name, + }) + : t("fileManager.uploadedFolderFiles", { count: files.length }), ); } else if (failed.length === files.length) { toast.error(t("fileManager.failedToUploadFile")); @@ -1277,13 +1323,22 @@ function FileManagerContent({ } handleRefreshDirectory(); } catch (error) { - toast.dismiss(progressToast); + toast.dismiss(toastId); if (controller.signal.aborted) { toast.info(t("fileManager.localTransferCancelled")); handleRefreshDirectory(); return; } - toast.error(t("fileManager.failedToUploadFile")); + const message = error instanceof Error ? error.message : ""; + toast.error( + message.includes("connection") || message.includes("established") + ? t("fileManager.sshConnectionFailed", { + name: currentHost?.name, + ip: currentHost?.ip, + port: currentHost?.port, + }) + : t("fileManager.failedToUploadFile"), + ); } } @@ -1293,91 +1348,10 @@ function FileManagerContent({ return; } - await runWithConcurrency(Array.from(fileList), 3, handleUploadFile); - } - - async function handleUploadFile(file: File) { - if (!sshSessionId) return; - - const controller = new AbortController(); - const cancel = { - label: t("fileManager.cancel"), - onClick: () => controller.abort(), - }; - const progressToast = toast.loading( - t("fileManager.uploadingFile", { - name: file.name, - size: formatFileSize(file.size), - }), - { duration: Infinity, cancel }, + await uploadBatch( + Array.from(fileList).map((file) => ({ file, relativePath: file.name })), + [], ); - - const updateProgress = (p: { - chunkIndex: number; - totalChunks: number; - bytesSent: number; - totalBytes: number; - }) => { - if (controller.signal.aborted) return; - const percent = Math.min( - 100, - Math.round((p.bytesSent / p.totalBytes) * 100), - ); - toast.loading( - t("fileManager.uploadingChunk", { - name: file.name, - percent, - chunk: p.chunkIndex + 1, - total: p.totalChunks, - }), - { id: progressToast, duration: Infinity, cancel }, - ); - }; - - try { - await ensureSSHConnection(); - - await uploadSSHFile( - sshSessionId, - currentPath, - file.name, - file, - currentHost?.id, - undefined, - updateProgress, - controller.signal, - ); - controller.signal.throwIfAborted(); - - toast.dismiss(progressToast); - - toast.success( - t("fileManager.fileUploadedSuccessfully", { name: file.name }), - ); - handleRefreshDirectory(); - } catch (error: unknown) { - toast.dismiss(progressToast); - if (controller.signal.aborted) { - toast.info(t("fileManager.localTransferCancelled")); - handleRefreshDirectory(); - return; - } - const uploadErr = error instanceof Error ? error : null; - if ( - uploadErr?.message?.includes("connection") || - uploadErr?.message?.includes("established") - ) { - toast.error( - t("fileManager.sshConnectionFailed", { - name: currentHost?.name, - ip: currentHost?.ip, - port: currentHost?.port, - }), - ); - } else { - toast.error(t("fileManager.failedToUploadFile")); - } - } } async function handleDownloadFile(file: FileItem) { @@ -1828,6 +1802,115 @@ function FileManagerContent({ }); } + // Open editor sessions by remote path. Saves are chained per path (across + // reopen) so an older queued save can never land after a newer one. + const externalEditsRef = useRef( + new Map void }>(), + ); + const externalSavesRef = useRef(new Map>()); + const externalOpenTokensRef = useRef(new Map()); + const externalMountedRef = useRef(true); + + useEffect(() => { + externalMountedRef.current = true; + const edits = externalEditsRef.current; + return () => { + externalMountedRef.current = false; + for (const edit of edits.values()) { + edit.unsubscribe?.(); + void window.electronAPI?.closeExternalEditor?.(edit.editId); + } + edits.clear(); + }; + }, []); + + async function handleOpenExternally(file: FileItem) { + const api = window.electronAPI; + if (!api?.isElectron) { + toast.error(t("fileManager.externalEditorDesktopOnly")); + return; + } + const tokens = externalOpenTokensRef.current; + const token = (tokens.get(file.path) ?? 0) + 1; + tokens.set(file.path, token); + // A newer open of this path, or unmounting, makes this request stale. + const isStale = () => + !externalMountedRef.current || tokens.get(file.path) !== token; + try { + if (!(await confirmLargeFileOpen(file))) return; + await ensureSSHConnection(); + // Let saves from a previous session finish before reading the file back. + await externalSavesRef.current.get(file.path); + const data = await downloadSSHFile(sshSessionId!, file.path); + // Saves come back as UTF-8 text, so binary files would be corrupted. + let text: string; + try { + const bytes = Uint8Array.from(atob(data.content), (c) => + c.charCodeAt(0), + ); + text = new TextDecoder("utf-8", { + fatal: true, + ignoreBOM: true, + }).decode(bytes); + } catch { + toast.error(t("fileManager.externalEditorTextOnly")); + return; + } + const editorPath = await api + .getSetting?.("fileManager.externalEditorPath") + .catch(() => ""); + if (isStale()) return; + const result = await api.openExternalEditor({ + fileName: file.name, + content: text, + encoding: "utf8", + editorPath: + typeof editorPath === "string" && editorPath ? editorPath : null, + }); + if (!result.success || !result.editId) { + throw new Error(result.error || "Failed to open external editor"); + } + if (isStale()) { + void api.closeExternalEditor?.(result.editId); + return; + } + const previous = externalEditsRef.current.get(file.path); + if (previous) { + previous.unsubscribe?.(); + void api.closeExternalEditor?.(previous.editId); + externalEditsRef.current.delete(file.path); + } + const sessionId = sshSessionId!; + const saves = externalSavesRef.current; + const unsubscribe = api.onExternalEditorSaved?.((payload) => { + if (payload.editId !== result.editId) return; + const next = (saves.get(file.path) ?? Promise.resolve()).then(() => + writeSSHFile(sessionId, file.path, payload.content).then( + () => void toast.success(t("fileManager.fileSavedSuccessfully")), + (error: { + message?: string; + response?: { data?: { needsSudo?: boolean } }; + }) => + void toast.error( + error.response?.data?.needsSudo + ? t("fileManager.permissionDenied") + : `${t("fileManager.failedToSaveFile")}: ${error.message || t("fileManager.unknownError")}`, + ), + ), + ); + saves.set(file.path, next); + }); + externalEditsRef.current.set(file.path, { + editId: result.editId, + unsubscribe, + }); + toast.success(t("fileManager.externalEditorOpened")); + } catch (error) { + toast.error( + `${t("fileManager.failedToOpenExternalEditor")}: ${error instanceof Error && error.message ? error.message : t("fileManager.unknownError")}`, + ); + } + } async function handleFileOpen(file: FileItem) { if (file.type === "directory") { markAdaptiveResourceUsed("network", "directory-list"); @@ -3618,6 +3701,7 @@ function FileManagerContent({ .forEach(handleDownloadFile) } onPreview={handleFileOpen} + onOpenExternally={handleOpenExternally} onRename={handleRenameFile} onCopy={handleCopyFiles} onCut={handleCutFiles} diff --git a/src/frontend/FileManagerContextMenu.tsx b/src/frontend/FileManagerContextMenu.tsx index 1c573eb..b2da396 100644 --- a/src/frontend/FileManagerContextMenu.tsx +++ b/src/frontend/FileManagerContextMenu.tsx @@ -20,6 +20,7 @@ import { FileArchive, ArrowRightLeft, Link, + ExternalLink, } from "lucide-react"; import { useTranslation } from "@termix-ssh/plugin-sdk/frontend"; import { Kbd, KbdKey, KbdSeparator } from "@termix-ssh/plugin-sdk/ui"; @@ -56,6 +57,7 @@ interface ContextMenuProps { onRefresh?: () => void; onPaste?: () => void; onPreview?: (file: FileItem) => void; + onOpenExternally?: (file: FileItem) => void; hasClipboard?: boolean; onDragToDesktop?: () => void; onOpenTerminal?: (path: string) => void; @@ -100,6 +102,7 @@ export function FileManagerContextMenu({ onRefresh, onPaste, onPreview, + onOpenExternally, hasClipboard = false, onDragToDesktop, onOpenTerminal, @@ -254,6 +257,14 @@ export function FileManagerContextMenu({ }); } + if (hasFiles && onOpenExternally) { + menuItems.push({ + icon: , + label: t("fileManager.openExternalEditor"), + action: () => onOpenExternally(files[0]), + disabled: !isSingleFile || files[0].type !== "file", + }); + } if (hasFiles && onDownload) { menuItems.push({ icon: , diff --git a/src/frontend/FileManagerGrid.tsx b/src/frontend/FileManagerGrid.tsx index d29213f..e5713b9 100644 --- a/src/frontend/FileManagerGrid.tsx +++ b/src/frontend/FileManagerGrid.tsx @@ -392,9 +392,12 @@ export function FileManagerGrid({ const handleEditKeyDown = (e: React.KeyboardEvent) => { if (e.key === "Enter") { e.preventDefault(); + // Stop the global grid Enter-to-open shortcut firing after the input unmounts. + e.stopPropagation(); handleEditConfirm(); } else if (e.key === "Escape") { e.preventDefault(); + e.stopPropagation(); handleEditCancel(); } }; @@ -1675,9 +1678,12 @@ function CreateIntentGridItem({ const handleKeyDown = (e: React.KeyboardEvent) => { if (e.key === "Enter") { e.preventDefault(); + // Stop the global grid Enter-to-open shortcut firing after the input unmounts. + e.stopPropagation(); commit(inputName.trim()); } else if (e.key === "Escape") { e.preventDefault(); + e.stopPropagation(); if (doneRef.current) return; doneRef.current = true; onCancel?.(); @@ -1757,9 +1763,12 @@ function CreateIntentListItem({ const handleKeyDown = (e: React.KeyboardEvent) => { if (e.key === "Enter") { e.preventDefault(); + // Stop the global grid Enter-to-open shortcut firing after the input unmounts. + e.stopPropagation(); commit(inputName.trim()); } else if (e.key === "Escape") { e.preventDefault(); + e.stopPropagation(); if (doneRef.current) return; doneRef.current = true; onCancel?.(); diff --git a/src/frontend/components/UploadProgressToast.tsx b/src/frontend/components/UploadProgressToast.tsx new file mode 100644 index 0000000..5a68bba --- /dev/null +++ b/src/frontend/components/UploadProgressToast.tsx @@ -0,0 +1,65 @@ +import type { TranslateFn } from "@termix-ssh/plugin-sdk/frontend"; +import { ArrowUpFromLine } from "lucide-react"; +import { formatTransferMbPerSec } from "../api/transfer-api"; +import { formatFileSize } from "../file-manager-utils"; +import { TransferProgressBar } from "./TransferProgressBar"; + +interface UploadProgressToastProps { + t: TranslateFn; + doneFiles: number; + totalFiles: number; + loaded: number; + total: number; + mbPerSec?: number; + onCancel: () => void; +} + +export function UploadProgressToast({ + t, + doneFiles, + totalFiles, + loaded, + total, + mbPerSec, + onCancel, +}: UploadProgressToastProps) { + const percent = + total > 0 ? Math.min(100, Math.round((loaded / total) * 100)) : undefined; + const speed = formatTransferMbPerSec(mbPerSec); + const title = t("fileManager.uploadingFolderFiles", { count: totalFiles }); + + return ( +
+
+

+ + {title} +

+ +
+ +
+ + {t("fileManager.uploadFilesDone", { + done: doneFiles, + total: totalFiles, + })} + {" ยท "} + {formatFileSize(loaded)} / {formatFileSize(total)} + + + {speed || "0 MB/s"} + +
+
+ ); +} diff --git a/tests/backend/sftp-pipelined-write.test.ts b/tests/backend/sftp-pipelined-write.test.ts new file mode 100644 index 0000000..a6f8e30 --- /dev/null +++ b/tests/backend/sftp-pipelined-write.test.ts @@ -0,0 +1,102 @@ +import { PassThrough } from "node:stream"; +import { describe, expect, it, vi } from "vitest"; +import { pipelineWriteToSftp } from "../../src/backend/sftp-pipelined-write"; + +type Ack = (err?: Error) => void; + +function fakeSftp(opts: { openDelay?: boolean; failWriteAt?: number } = {}) { + const file = Buffer.alloc(8 * 1024 * 1024); + const pending: Ack[] = []; + let writes = 0; + let maxInFlight = 0; + let openCb: ((e: Error | undefined, h: Buffer) => void) | undefined; + const close = vi.fn((_h: Buffer, cb: (e?: Error) => void) => cb()); + const sftp = { + open: vi.fn((_p: string, _f: string, cb: typeof openCb) => { + if (opts.openDelay) openCb = cb; + else cb!(undefined, Buffer.from("h")); + }), + write: vi.fn( + ( + _h: Buffer, + buf: Buffer, + off: number, + len: number, + pos: number, + cb: Ack, + ) => { + buf.copy(file, pos, off, off + len); + const failing = opts.failWriteAt === writes++; + pending.push(() => cb(failing ? new Error("write failed") : undefined)); + maxInFlight = Math.max(maxInFlight, pending.length); + }, + ), + close, + }; + // Acknowledge in reverse order to prove offsets do not depend on ack order. + const flush = () => { + while (pending.length) pending.pop()!(); + }; + return { + sftp: sftp as never, + close, + file, + flush, + finishOpen: () => openCb!(undefined, Buffer.from("h")), + maxInFlight: () => maxInFlight, + }; +} + +const tick = () => new Promise((r) => setImmediate(r)); + +describe("pipelineWriteToSftp", () => { + it("writes exact bytes with out-of-order acks and bounded concurrency", async () => { + const f = fakeSftp(); + const src = new PassThrough(); + const data = Buffer.from( + Array.from({ length: 6_000_000 }, (_, i) => i % 251), + ); + const { done } = pipelineWriteToSftp(f.sftp, "/x", src); + src.end(data); + for (let i = 0; i < 200 && f.close.mock.calls.length === 0; i++) { + await tick(); + f.flush(); + } + await expect(done).resolves.toBe(data.length); + expect(f.file.subarray(0, data.length).equals(data)).toBe(true); + expect(f.maxInFlight()).toBeLessThanOrEqual(64); + expect(f.close).toHaveBeenCalledTimes(1); + }); + + it("rejects, drains the source and closes once on a write error", async () => { + const f = fakeSftp({ failWriteAt: 2 }); + const src = new PassThrough(); + const { done } = pipelineWriteToSftp(f.sftp, "/x", src); + src.write(Buffer.alloc(512 * 1024)); + await tick(); + f.flush(); + await expect(done).rejects.toThrow("write failed"); + expect(src.isPaused()).toBe(false); + expect(f.close).toHaveBeenCalledTimes(1); + }); + + it("rejects instead of throwing when the source errors before open finishes", async () => { + const f = fakeSftp({ openDelay: true }); + const src = new PassThrough(); + const { done } = pipelineWriteToSftp(f.sftp, "/x", src); + src.destroy(new Error("truncated")); + await expect(done).rejects.toThrow("truncated"); + f.finishOpen(); + expect(f.close).toHaveBeenCalledTimes(1); // late handle is not leaked + }); + + it("aborts before open without leaking the handle", async () => { + const f = fakeSftp({ openDelay: true }); + const src = new PassThrough(); + const { done, abort } = pipelineWriteToSftp(f.sftp, "/x", src); + abort(); + await expect(done).rejects.toThrow("aborted"); + f.finishOpen(); + expect(f.close).toHaveBeenCalledTimes(1); + }); +}); diff --git a/tests/backend/upload-cancellation.test.ts b/tests/backend/upload-cancellation.test.ts index 5be67e3..f5f6395 100644 --- a/tests/backend/upload-cancellation.test.ts +++ b/tests/backend/upload-cancellation.test.ts @@ -114,6 +114,12 @@ describe("multipart upload disconnects", () => { const remote = { ...sftp(), unlink: vi.fn((_path, callback) => callback(null)), + // Multipart uploads use pipelined SFTP writes (open/write/close). + open: vi.fn((_p: string, _f: string, cb: (e: null, h: Buffer) => void) => + cb(null, Buffer.from("h")), + ), + write: vi.fn(), + close: vi.fn((_h: Buffer, cb: () => void) => cb()), }; getSessionSftp.mockResolvedValue(remote); const { req, res, start } = setup("/uploadFileStream"); @@ -138,11 +144,15 @@ describe("multipart upload disconnects", () => { ].join("\r\n"), ); await new Promise((resolve) => setImmediate(resolve)); - expect(remote.createWriteStream).toHaveBeenCalledWith("/a.txt"); + expect(remote.open).toHaveBeenCalledWith( + "/a.txt", + "w", + expect.any(Function), + ); req.emit("close"); - expect(remote.stream.destroyed).toBe(false); + expect(remote.close).not.toHaveBeenCalled(); res.emit("close"); - expect(remote.stream.destroyed).toBe(true); + expect(remote.close).toHaveBeenCalled(); expect(remote.unlink).toHaveBeenCalledWith("/a.txt", expect.any(Function)); expect(res.json).not.toHaveBeenCalled(); }); diff --git a/tests/frontend/FileManagerGrid.test.tsx b/tests/frontend/FileManagerGrid.test.tsx index d2bb223..cedef26 100644 --- a/tests/frontend/FileManagerGrid.test.tsx +++ b/tests/frontend/FileManagerGrid.test.tsx @@ -89,6 +89,27 @@ describe("remote file rename", () => { rerender(); expect(screen.getByText(file.modified!)).toBeInTheDocument(); }); + + it("does not open the file when Enter confirms a rename", () => { + vi.spyOn(HTMLElement.prototype, "offsetParent", "get").mockReturnValue( + document.body, + ); + const onFileOpen = vi.fn(); + const onRename = vi.fn(); + render( + , + ); + const input = screen.getByRole("textbox"); + fireEvent.change(input, { target: { value: "renamed.txt" } }); + fireEvent.keyDown(input, { key: "Enter" }); + expect(onRename).toHaveBeenCalledWith(file, "renamed.txt"); + expect(onFileOpen).not.toHaveBeenCalled(); + }); }); describe("external file drops", () => { From 8f84e07f55448779211261c6e602e069c3b440d7 Mon Sep 17 00:00:00 2001 From: deanxbox Date: Sun, 18 Oct 2026 20:16:50 +0100 Subject: [PATCH 2/2] fix: address upload toast and large-file decoding review Replace the upload speed fallback with an invisible non-breaking space and reserve right-aligned space for the reading. Decode base64 in 3 MB slices into a preallocated byte array when opening files externally. Add round-trip tests for empty, small, and multi-slice inputs. --- src/frontend/FileManager.tsx | 10 ++++++---- src/frontend/components/UploadProgressToast.tsx | 4 ++-- src/frontend/file-manager-utils.ts | 13 +++++++++++++ tests/frontend/base64-to-bytes.test.ts | 13 +++++++++++++ 4 files changed, 34 insertions(+), 6 deletions(-) create mode 100644 tests/frontend/base64-to-bytes.test.ts diff --git a/src/frontend/FileManager.tsx b/src/frontend/FileManager.tsx index d8085d0..5a0f015 100644 --- a/src/frontend/FileManager.tsx +++ b/src/frontend/FileManager.tsx @@ -111,7 +111,11 @@ import type { PendingSudoOperation, SSHConnectionError, } from "./file-manager-types.ts"; -import { formatFileSize, runWithConcurrency } from "./file-manager-utils.ts"; +import { + base64ToBytes, + formatFileSize, + runWithConcurrency, +} from "./file-manager-utils.ts"; import { invalidateCachedFileList, peekCachedFileList, @@ -1845,9 +1849,7 @@ function FileManagerContent({ // Saves come back as UTF-8 text, so binary files would be corrupted. let text: string; try { - const bytes = Uint8Array.from(atob(data.content), (c) => - c.charCodeAt(0), - ); + const bytes = base64ToBytes(data.content); text = new TextDecoder("utf-8", { fatal: true, ignoreBOM: true, diff --git a/src/frontend/components/UploadProgressToast.tsx b/src/frontend/components/UploadProgressToast.tsx index 5a68bba..181e405 100644 --- a/src/frontend/components/UploadProgressToast.tsx +++ b/src/frontend/components/UploadProgressToast.tsx @@ -54,10 +54,10 @@ export function UploadProgressToast({ {formatFileSize(loaded)} / {formatFileSize(total)} - {speed || "0 MB/s"} + {speed || "\u00A0"} diff --git a/src/frontend/file-manager-utils.ts b/src/frontend/file-manager-utils.ts index 3b27a9d..d2992d0 100644 --- a/src/frontend/file-manager-utils.ts +++ b/src/frontend/file-manager-utils.ts @@ -33,3 +33,16 @@ export async function runWithConcurrency( }), ); } + +/** Decodes base64 in slices so no full-size binary string is ever built. */ +export function base64ToBytes(b64: string): Uint8Array { + const SLICE = 3 * 1024 * 1024; // multiple of 4 chars per slice stays aligned + const pad = b64.endsWith("==") ? 2 : b64.endsWith("=") ? 1 : 0; + const out = new Uint8Array(Math.floor((b64.length * 3) / 4) - pad); + let offset = 0; + for (let i = 0; i < b64.length; i += SLICE) { + const bin = atob(b64.slice(i, i + SLICE)); + for (let j = 0; j < bin.length; j++) out[offset++] = bin.charCodeAt(j); + } + return out; +} diff --git a/tests/frontend/base64-to-bytes.test.ts b/tests/frontend/base64-to-bytes.test.ts new file mode 100644 index 0000000..e515c85 --- /dev/null +++ b/tests/frontend/base64-to-bytes.test.ts @@ -0,0 +1,13 @@ +import { describe, expect, it } from "vitest"; +import { base64ToBytes } from "../../src/frontend/file-manager-utils"; + +describe("base64ToBytes", () => { + it.each([0, 1, 2, 3, 4, 100, 3 * 1024 * 1024 + 5])( + "round-trips %i bytes across slice boundaries", + (n) => { + const bytes = Uint8Array.from({ length: n }, (_, i) => (i * 7) % 256); + const b64 = Buffer.from(bytes).toString("base64"); + expect(base64ToBytes(b64)).toEqual(bytes); + }, + ); +});