diff --git a/OpenBitFun-Installer/src-tauri/Cargo.toml b/OpenBitFun-Installer/src-tauri/Cargo.toml index d54b92c337..3af7052730 100644 --- a/OpenBitFun-Installer/src-tauri/Cargo.toml +++ b/OpenBitFun-Installer/src-tauri/Cargo.toml @@ -15,10 +15,16 @@ path = "src/main.rs" [build-dependencies] tauri-build = { version = "=2.6.3", features = [] } +# This standalone app does not commit a Cargo.lock. Keep the Tauri build-time +# crates aligned with the patched 2.11 runtime instead of resolving 2.7/2.10. +tauri-codegen = "=2.6.3" zip = "0.6" [dependencies] tauri = { version = "=2.11.5", features = [] } +tauri-macros = "=2.6.3" +tauri-plugin = "=2.6.3" +tauri-utils = "=2.9.3" tauri-plugin-dialog = "=2.7.3" serde = { version = "1", features = ["derive"] } serde_json = "1" diff --git a/docs/architecture/remote-workspace-transport.md b/docs/architecture/remote-workspace-transport.md index 18c368cca9..1fa507c2a7 100644 --- a/docs/architecture/remote-workspace-transport.md +++ b/docs/architecture/remote-workspace-transport.md @@ -183,6 +183,29 @@ through the destination to preserve existing links and permissions. The final write is not an atomic transaction against other writers; interruption during that phase can have a partial or unknown outcome. +Both providers publish an upload only after the transfer completed: the SFTP +path streams into a same-directory `-upload-.tmp` sibling +and renames it over the destination, the container command does the same after +its size check, and a downloaded file is staged beside its local destination +the same way. A failed or cancelled transfer therefore leaves any previous +destination unchanged, and staging temporaries are removed when the owning side +can still reach the path. For directory transfers, files already published and +directories already created remain when a later entry fails or is stopped. +Because a published upload replaces its destination +rather than writing through it, the result takes the staging file's mode and +ownership, and a replaced symlink or hard link is not followed; that is what +distinguishes a transfer from a workspace tool write above. A hard kill between +staging and commit can leave one orphaned temporary, which the container +commands sweep by age and the SFTP path leaves to the user. + +The desktop upload and download commands opt out of the shared 30-second +controller deadline when their caller provides a progress and stop surface. +Every SFTP request retains its own response timeout, the SSH transport drops a +stalled connection, and the transfer ends when it completes, fails, or is +stopped. File reads, writes, listings, and transfers without a stop surface keep +the default controller deadline. A short deadline on a stoppable transfer only +reported a slow-but-healthy operation as failed while the host kept working. + Directory and stat records use NUL-separated fields. File names containing newlines or the delimiters used by older implementations remain round-trippable. The records are decoded only after the full byte stream is assembled; invalid diff --git a/src/apps/desktop/src/api/ssh_api.rs b/src/apps/desktop/src/api/ssh_api.rs index d89012c158..555c6ac74b 100644 --- a/src/apps/desktop/src/api/ssh_api.rs +++ b/src/apps/desktop/src/api/ssh_api.rs @@ -3,7 +3,10 @@ //! Tauri commands for SSH connection management and remote file operations. use serde::Serialize; +use std::collections::HashMap; +use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::{Arc, Mutex}; use std::time::Instant; use tauri::{Emitter, State}; @@ -387,6 +390,247 @@ pub async fn remote_rename( .map_err(|e| e.to_string()) } +/// Bytes read or written per transfer step. Matches the chunk the remote file +/// service uses, so one progress step is one protocol round trip. +const TRANSFER_CHUNK_BYTES: usize = 256 * 1024; + +/// Progress events are throttled to one per interval, plus the terminal event. +const TRANSFER_PROGRESS_INTERVAL_MS: u128 = 100; + +/// Cancellation and cooperative abort share one message so a user-initiated +/// stop is recognizable on every transfer path. +const TRANSFER_CANCELLED: &str = "Transfer cancelled"; + +/// Cancellation registry shared with the `cancel_transfer` command. +type TransferRegistry = Arc>>>; + +/// One in-flight transfer's cancellation slot. +/// +/// The entry is removed on every exit path — including `?` early returns and +/// panics — so a failed or rejected transfer cannot leave a stale flag behind +/// that would abort a later transfer reusing the same id. +struct ActiveTransfer { + registry: TransferRegistry, + transfer_id: String, + cancel_flag: Arc, +} + +impl ActiveTransfer { + fn register(registry: &TransferRegistry, transfer_id: &str) -> Result { + let cancel_flag = Arc::new(AtomicBool::new(false)); + registry + .lock() + .map_err(|error| error.to_string())? + .insert(transfer_id.to_string(), cancel_flag.clone()); + Ok(Self { + registry: registry.clone(), + transfer_id: transfer_id.to_string(), + cancel_flag, + }) + } + + fn is_cancelled(&self) -> bool { + self.cancel_flag.load(Ordering::Relaxed) + } +} + +impl Drop for ActiveTransfer { + fn drop(&mut self) { + let Ok(mut registry) = self.registry.lock() else { + return; + }; + // A reused id may already belong to a newer transfer; only this + // transfer's own flag may be removed. + if registry + .get(&self.transfer_id) + .is_some_and(|flag| Arc::ptr_eq(flag, &self.cancel_flag)) + { + registry.remove(&self.transfer_id); + } + } +} + +/// Emit a throttled progress event; `terminal` always emits. +fn emit_transfer_progress( + app_handle: &tauri::AppHandle, + event: &str, + payload: impl Serialize + Clone, + last_emit: &mut Instant, + terminal: bool, +) { + let now = Instant::now(); + if !terminal && now.duration_since(*last_emit).as_millis() < TRANSFER_PROGRESS_INTERVAL_MS { + return; + } + *last_emit = now; + let _ = app_handle.emit(event, payload); +} + +fn emit_download_progress( + app_handle: &tauri::AppHandle, + transfer_id: &str, + downloaded: u64, + total: u64, + last_emit: &mut Instant, + terminal: bool, +) { + emit_transfer_progress( + app_handle, + "download_progress", + DownloadProgressPayload { + transfer_id: transfer_id.to_string(), + downloaded, + total, + }, + last_emit, + terminal, + ); +} + +/// Local staging for a streamed remote download. +/// +/// Bytes land in a private temporary file beside the destination and replace it +/// only after the transfer completed, so a cancelled or failed download never +/// truncates an existing local file. This mirrors the controller-local peer +/// sink in [`crate::api::local_file_download`]. +struct LocalDownloadStaging { + file: tokio::fs::File, + temp_path: tempfile::TempPath, + destination: PathBuf, +} + +impl LocalDownloadStaging { + async fn begin(destination: &Path) -> Result { + let parent = destination + .parent() + .filter(|parent| !parent.as_os_str().is_empty()) + .unwrap_or_else(|| Path::new(".")); + tokio::fs::create_dir_all(parent).await.map_err(|error| { + format!( + "Failed to prepare local download directory '{}': {}", + parent.display(), + error + ) + })?; + let (file, temp_path) = tempfile::Builder::new() + .prefix(".openbitfun-download-") + .tempfile_in(parent) + .map_err(|error| { + format!( + "Failed to stage local download beside '{}': {}", + destination.display(), + error + ) + })? + .into_parts(); + Ok(Self { + file: tokio::fs::File::from_std(file), + temp_path, + destination: destination.to_path_buf(), + }) + } + + async fn write(&mut self, chunk: &[u8]) -> Result<(), String> { + use tokio::io::AsyncWriteExt; + self.file.write_all(chunk).await.map_err(|error| { + format!( + "Failed to write local download '{}': {}", + self.destination.display(), + error + ) + }) + } + + async fn publish(self) -> Result<(), String> { + let Self { + file, + temp_path, + destination, + } = self; + file.sync_all().await.map_err(|error| { + format!( + "Failed to flush local download '{}': {}", + destination.display(), + error + ) + })?; + drop(file); + let failure_path = destination.clone(); + tokio::task::spawn_blocking(move || temp_path.persist(&destination)) + .await + .map_err(|error| error.to_string())? + .map_err(|error| { + format!( + "Failed to publish local download '{}': {}", + failure_path.display(), + error + ) + }) + } +} + +/// Stream one remote file into a staged local file and publish it. +/// +/// `on_progress` receives the bytes read for this file after every chunk; the +/// caller owns throttling and the cumulative total it reports to the frontend. +/// The remote reader bounds every protocol request, so a cancelled transfer +/// stops at the next chunk boundary instead of waiting for the whole file. +async fn download_remote_file_to_local( + remote_fs: &openbitfun_core::service::remote_ssh::RemoteFileService, + connection_id: &str, + remote_path: &str, + destination: &Path, + transfer: &ActiveTransfer, + on_progress: &mut impl FnMut(u64), +) -> Result { + let mut reader = remote_fs + .open_read(connection_id, remote_path) + .await + .map_err(|error| format!("Failed to open remote file '{remote_path}': {error}"))?; + let mut staging = LocalDownloadStaging::begin(destination).await?; + let mut buffer = vec![0_u8; TRANSFER_CHUNK_BYTES]; + let mut downloaded = 0_u64; + loop { + if transfer.is_cancelled() { + return Err(TRANSFER_CANCELLED.to_string()); + } + use tokio::io::AsyncReadExt; + let read = reader + .read(&mut buffer) + .await + .map_err(|error| format!("Failed to read remote file '{remote_path}': {error}"))?; + if read == 0 { + break; + } + staging.write(&buffer[..read]).await?; + downloaded = downloaded.saturating_add(read as u64); + on_progress(downloaded); + } + // Stop can arrive during the final read (including an empty file). Do not + // replace a destination after that stop just because the stream reached EOF. + if transfer.is_cancelled() { + return Err(TRANSFER_CANCELLED.to_string()); + } + staging.publish().await?; + Ok(downloaded) +} + +/// Remote file size used only to keep the reported percentage sane; the actual +/// byte count always comes from the stream. +async fn remote_declared_size( + remote_fs: &openbitfun_core::service::remote_ssh::RemoteFileService, + connection_id: &str, + remote_path: &str, +) -> u64 { + remote_fs + .workspace_metadata(connection_id, remote_path, true) + .await + .ok() + .flatten() + .and_then(|metadata| metadata.size) + .unwrap_or(0) +} + /// Payload emitted via `download_progress` events during a remote download. #[derive(Debug, Clone, Serialize)] #[serde(rename_all = "camelCase")] @@ -396,19 +640,20 @@ pub struct DownloadProgressPayload { pub total: u64, } -/// Read a remote file or directory via SFTP and write it to a local path. +/// Read a remote file or directory and stream it to a local path. /// -/// If `remote_path` is a file, its bytes are read via SFTP and written to -/// `local_path` (binary-safe). If it is a directory, the directory tree is -/// recreated locally: subdirectories are created with `create_dir_all`, and -/// each file is read via SFTP and written to disk. +/// If `remote_path` is a file, its bytes are streamed to `local_path` +/// (binary-safe) through a staged temporary file that replaces the destination +/// only after the transfer completed. If it is a directory, the directory tree +/// is recreated locally: subdirectories are created with `create_dir_all`, and +/// each file is streamed the same way. /// /// Emits `download_progress` events with `{ transferId, downloaded, total }` -/// (bytes) during the SFTP read so the frontend can render a determinate -/// progress bar with speed display. The `transfer_id` lets the frontend -/// distinguish concurrent downloads and cancel individual transfers. Events -/// are throttled to at most one per 100 ms (plus a guaranteed final event) to -/// avoid flooding the webview. +/// (bytes) during the read so the frontend can render a determinate progress +/// bar with speed display. The `transfer_id` lets the frontend distinguish +/// concurrent downloads and cancel individual transfers. Events are throttled +/// to at most one per 100 ms (plus a guaranteed final event) to avoid flooding +/// the webview. #[tauri::command] pub async fn remote_download_to_local_path( app_handle: tauri::AppHandle, @@ -418,14 +663,9 @@ pub async fn remote_download_to_local_path( local_path: String, transfer_id: String, ) -> Result<(), String> { + let transfer = ActiveTransfer::register(&state.active_transfers, &transfer_id)?; let remote_fs = state.get_remote_file_service_async().await?; - - // Register a cancellation flag for this transfer. - let cancel_flag = std::sync::Arc::new(AtomicBool::new(false)); - { - let mut map = state.active_transfers.lock().map_err(|e| e.to_string())?; - map.insert(transfer_id.clone(), cancel_flag.clone()); - } + let mut last_emit = Instant::now(); // Check if the remote path is a directory. let is_dir = remote_fs @@ -434,65 +674,48 @@ pub async fn remote_download_to_local_path( .map_err(|e| e.to_string())?; if is_dir { - let dir_result = download_directory_from_remote( + return download_directory_from_remote( &app_handle, &state, &connection_id, &remote_path, &local_path, &transfer_id, - &cancel_flag, + &transfer, + &mut last_emit, ) .await; - // Clean up the cancellation flag. - { - let mut map = state.active_transfers.lock().map_err(|e| e.to_string())?; - map.remove(&transfer_id); - } - return dir_result; } - // Regular file: read via SFTP with progress. - let mut last_emit = Instant::now(); - let result = remote_fs - .read_file_with_progress(&connection_id, &remote_path, &mut |downloaded, total| { - // Throttle: emit at most every 100 ms, plus the final 100% event. - let now = Instant::now(); - if downloaded >= total || now.duration_since(last_emit).as_millis() >= 100 { - let _ = app_handle.emit( - "download_progress", - DownloadProgressPayload { - transfer_id: transfer_id.clone(), - downloaded, - total, - }, - ); - last_emit = now; - } - // Return false to abort the read if cancelled. - !cancel_flag.load(Ordering::Relaxed) - }) - .await; - - // Clean up the cancellation flag. - { - let mut map = state.active_transfers.lock().map_err(|e| e.to_string())?; - map.remove(&transfer_id); - } - - let bytes = result.map_err(|e| e.to_string())?; - - tokio::task::spawn_blocking(move || { - let path = std::path::Path::new(&local_path); - if let Some(parent) = path.parent() { - if !parent.as_os_str().is_empty() { - std::fs::create_dir_all(parent).map_err(|e| e.to_string())?; - } - } - std::fs::write(path, &bytes).map_err(|e| e.to_string()) - }) - .await - .map_err(|e| e.to_string())? + let declared_total = remote_declared_size(&remote_fs, &connection_id, &remote_path).await; + let downloaded = download_remote_file_to_local( + &remote_fs, + &connection_id, + &remote_path, + Path::new(&local_path), + &transfer, + &mut |downloaded| { + // A file that grew while it was being read must not report over 100%. + emit_download_progress( + &app_handle, + &transfer_id, + downloaded, + declared_total.max(downloaded), + &mut last_emit, + false, + ); + }, + ) + .await?; + emit_download_progress( + &app_handle, + &transfer_id, + downloaded, + declared_total.max(downloaded), + &mut last_emit, + true, + ); + Ok(()) } fn validate_remote_name_for_local_download(name: &str) -> Result<(), String> { @@ -568,9 +791,10 @@ fn local_download_name_key(name: &str) -> String { /// Recursively download a remote directory to a local path. /// /// Pre-scans the remote tree to determine total file size, then walks the tree -/// and downloads each file with chunked progress reporting. Emits cumulative +/// and streams each file through a staged local file. Emits cumulative /// `download_progress` events so the frontend can show overall directory /// download progress. +#[allow(clippy::too_many_arguments)] async fn download_directory_from_remote( app_handle: &tauri::AppHandle, state: &State<'_, AppState>, @@ -578,12 +802,13 @@ async fn download_directory_from_remote( remote_dir: &str, local_dir: &str, transfer_id: &str, - cancel_flag: &std::sync::Arc, + transfer: &ActiveTransfer, + last_emit: &mut Instant, ) -> Result<(), String> { let remote_fs = state.get_remote_file_service_async().await?; // Create the top-level local directory. - let local_dir_path = std::path::PathBuf::from(local_dir); + let local_dir_path = PathBuf::from(local_dir); tokio::task::spawn_blocking(move || { std::fs::create_dir_all(&local_dir_path).map_err(|e| e.to_string()) }) @@ -594,11 +819,17 @@ async fn download_directory_from_remote( let mut total_bytes: u64 = 0; let mut scan_stack = vec![remote_dir.to_string()]; while let Some(current) = scan_stack.pop() { + if transfer.is_cancelled() { + return Err(TRANSFER_CANCELLED.to_string()); + } let entries = remote_fs .read_dir(connection_id, ¤t) .await .map_err(|e| e.to_string())?; for entry in entries { + if transfer.is_cancelled() { + return Err(TRANSFER_CANCELLED.to_string()); + } validate_remote_name_for_local_download(&entry.name)?; if entry.is_symlink { return Err(format!( @@ -615,15 +846,14 @@ async fn download_directory_from_remote( } let mut downloaded: u64 = 0; - let mut last_emit = Instant::now(); // Walk the remote directory tree. - let mut stack: Vec<(String, std::path::PathBuf)> = - vec![(remote_dir.to_string(), std::path::PathBuf::from(local_dir))]; + let mut stack: Vec<(String, PathBuf)> = + vec![(remote_dir.to_string(), PathBuf::from(local_dir))]; while let Some((remote_current, local_current)) = stack.pop() { - if cancel_flag.load(Ordering::Relaxed) { - return Err("Transfer cancelled".to_string()); + if transfer.is_cancelled() { + return Err(TRANSFER_CANCELLED.to_string()); } let entries = remote_fs @@ -659,56 +889,39 @@ async fn download_directory_from_remote( stack.push((remote_child, local_child)); } else { let base_downloaded = downloaded; - let bytes = remote_fs - .read_file_with_progress(connection_id, &remote_child, &mut |read_bytes, _| { - let cumulative = base_downloaded + read_bytes; - let now = Instant::now(); - if cumulative >= total_bytes - || now.duration_since(last_emit).as_millis() >= 100 - { - let _ = app_handle.emit( - "download_progress", - DownloadProgressPayload { - transfer_id: transfer_id.to_string(), - downloaded: cumulative, - total: total_bytes, - }, - ); - last_emit = now; - } - !cancel_flag.load(Ordering::Relaxed) - }) - .await - .map_err(|e| e.to_string())?; - - let file_size = bytes.len() as u64; - let local_child_write = local_child.clone(); - tokio::task::spawn_blocking(move || { - if let Some(parent) = local_child_write.parent() { - if !parent.as_os_str().is_empty() { - std::fs::create_dir_all(parent).map_err(|e| e.to_string())?; - } - } - std::fs::write(&local_child_write, &bytes).map_err(|e| e.to_string()) - }) - .await - .map_err(|e| e.to_string())??; - - downloaded += file_size; + let file_size = download_remote_file_to_local( + &remote_fs, + connection_id, + &remote_child, + &local_child, + transfer, + &mut |file_bytes| { + let cumulative = base_downloaded.saturating_add(file_bytes); + emit_download_progress( + app_handle, + transfer_id, + cumulative, + total_bytes.max(cumulative), + last_emit, + false, + ); + }, + ) + .await?; + + downloaded = base_downloaded.saturating_add(file_size); + emit_download_progress( + app_handle, + transfer_id, + downloaded, + total_bytes.max(downloaded), + last_emit, + true, + ); } } } - // Final progress event. - let _ = app_handle.emit( - "download_progress", - DownloadProgressPayload { - transfer_id: transfer_id.to_string(), - downloaded: total_bytes, - total: total_bytes, - }, - ); - Ok(()) } @@ -728,6 +941,27 @@ pub struct UploadProgressPayload { pub total: u64, } +fn emit_upload_progress( + app_handle: &tauri::AppHandle, + transfer_id: &str, + uploaded: u64, + total: u64, + last_emit: &mut Instant, + terminal: bool, +) { + emit_transfer_progress( + app_handle, + "upload_progress", + UploadProgressPayload { + transfer_id: transfer_id.to_string(), + uploaded, + total, + }, + last_emit, + terminal, + ); +} + /// Recursively scan a local directory and return the total size of all /// regular files in bytes. Used for pre-scanning before directory upload /// so that overall progress can be reported. @@ -768,18 +1002,22 @@ fn scan_directory_total_size(dir: &std::path::Path) -> Result { Ok(total) } -/// Upload a local file or directory tree to a remote path via SFTP. +/// Upload a local file or directory tree to a remote path. +/// +/// If `local_path` is a file, its bytes are streamed to `remote_path` without +/// buffering the file in memory. If it is a directory, the directory tree is +/// recreated on the remote side: subdirectories are created with +/// `create_dir_all`, and each file is streamed the same way. /// -/// If `local_path` is a file, its bytes are written to `remote_path`. If it is -/// a directory, the directory tree is recreated on the remote side: -/// subdirectories are created with `create_dir_all`, and each file is read -/// locally and written via SFTP. +/// An upload stages its bytes on the remote side and publishes them only after +/// the transfer completed, so a cancelled or failed upload never truncates a +/// valid destination with partial content. /// /// Emits `upload_progress` events with `{ transferId, uploaded, total }` -/// (bytes) during the SFTP write so the frontend can render a determinate -/// progress bar with speed display. The `transfer_id` lets the frontend -/// distinguish concurrent uploads and cancel individual transfers. Events -/// are throttled to at most one per 100 ms (plus a guaranteed final event). +/// (bytes) during the write so the frontend can render a determinate progress +/// bar with speed display. The `transfer_id` lets the frontend distinguish +/// concurrent uploads and cancel individual transfers. Events are throttled to +/// at most one per 100 ms (plus a guaranteed final event). #[tauri::command] pub async fn remote_upload_from_local_path( app_handle: tauri::AppHandle, @@ -789,6 +1027,7 @@ pub async fn remote_upload_from_local_path( remote_path: String, transfer_id: String, ) -> Result { + let transfer = ActiveTransfer::register(&state.active_transfers, &transfer_id)?; let local_path = std::path::Path::new(&local_path); let local_metadata = std::fs::symlink_metadata(local_path).map_err(|error| { format!( @@ -810,76 +1049,45 @@ pub async fn remote_upload_from_local_path( )); } - // Register a cancellation flag for this transfer. - let cancel_flag = std::sync::Arc::new(AtomicBool::new(false)); - { - let mut map = state.active_transfers.lock().map_err(|e| e.to_string())?; - map.insert(transfer_id.clone(), cancel_flag.clone()); - } - // A directory needs to be walked locally and recreated on the remote side. if local_path.is_dir() { - let dir_result = upload_directory_to_remote( + upload_directory_to_remote( &app_handle, &state, &connection_id, local_path, &remote_path, &transfer_id, - &cancel_flag, + &transfer, ) - .await; - // Clean up the cancellation flag. - { - let mut map = state.active_transfers.lock().map_err(|e| e.to_string())?; - map.remove(&transfer_id); - } - dir_result?; + .await?; return Ok(RemoteUploadResult { was_directory: true, }); } - // Regular file: read locally, write via SFTP with progress. - let local_path_owned = local_path.to_path_buf(); - let bytes = tokio::task::spawn_blocking(move || { - std::fs::read(&local_path_owned).map_err(|e| e.to_string()) - }) - .await - .map_err(|e| e.to_string())??; - + // Regular file: stream it to the remote without buffering it in memory. let remote_fs = state.get_remote_file_service_async().await?; let mut last_emit = Instant::now(); - let write_result = remote_fs - .write_file_with_progress( + remote_fs + .write_file_from_local_path_with_progress( &connection_id, &remote_path, - &bytes, + local_path, &mut |written, total| { - let now = Instant::now(); - if written >= total || now.duration_since(last_emit).as_millis() >= 100 { - let _ = app_handle.emit( - "upload_progress", - UploadProgressPayload { - transfer_id: transfer_id.clone(), - uploaded: written, - total, - }, - ); - last_emit = now; - } - !cancel_flag.load(Ordering::Relaxed) + emit_upload_progress( + &app_handle, + &transfer_id, + written, + total, + &mut last_emit, + written >= total, + ); + !transfer.is_cancelled() }, ) - .await; - - // Clean up the cancellation flag. - { - let mut map = state.active_transfers.lock().map_err(|e| e.to_string())?; - map.remove(&transfer_id); - } - - write_result.map_err(|e| e.to_string())?; + .await + .map_err(|e| e.to_string())?; Ok(RemoteUploadResult { was_directory: false, @@ -899,7 +1107,7 @@ async fn upload_directory_to_remote( local_dir: &std::path::Path, remote_dir: &str, transfer_id: &str, - cancel_flag: &std::sync::Arc, + transfer: &ActiveTransfer, ) -> Result<(), String> { let remote_fs = state.get_remote_file_service_async().await?; @@ -936,8 +1144,8 @@ async fn upload_directory_to_remote( .map_err(|e| e.to_string())??; for entry in entries { - if cancel_flag.load(Ordering::Relaxed) { - return Err("Transfer cancelled".to_string()); + if transfer.is_cancelled() { + return Err(TRANSFER_CANCELLED.to_string()); } let entry_path = entry.path(); let file_name = entry.file_name().into_string().map_err(|name| { @@ -966,42 +1174,37 @@ async fn upload_directory_to_remote( .map_err(|e| e.to_string())?; stack.push((entry_path, remote_child)); } else if file_type.is_file() { - let local_file = entry_path.clone(); - let bytes = tokio::task::spawn_blocking(move || { - std::fs::read(&local_file).map_err(|e| e.to_string()) - }) - .await - .map_err(|e| e.to_string())??; - - let file_size = bytes.len() as u64; let base_uploaded = uploaded; - remote_fs - .write_file_with_progress( + let file_size = remote_fs + .write_file_from_local_path_with_progress( connection_id, &remote_child, - &bytes, + &entry_path, &mut |written, _| { - let cumulative = base_uploaded + written; - let now = Instant::now(); - if cumulative >= total_bytes - || now.duration_since(last_emit).as_millis() >= 100 - { - let _ = app_handle.emit( - "upload_progress", - UploadProgressPayload { - transfer_id: transfer_id.to_string(), - uploaded: cumulative, - total: total_bytes, - }, - ); - last_emit = now; - } - !cancel_flag.load(Ordering::Relaxed) + let cumulative = base_uploaded.saturating_add(written); + emit_upload_progress( + app_handle, + transfer_id, + cumulative, + total_bytes.max(cumulative), + &mut last_emit, + false, + ); + !transfer.is_cancelled() }, ) .await .map_err(|e| e.to_string())?; - uploaded += file_size; + + uploaded = base_uploaded.saturating_add(file_size); + emit_upload_progress( + app_handle, + transfer_id, + uploaded, + total_bytes.max(uploaded), + &mut last_emit, + true, + ); } else { return Err(format!( "Unsupported local entry type in directory upload: '{}'", @@ -1211,7 +1414,69 @@ pub async fn ssh_list_remote_listening_ports( mod tests { use super::{ hydrate_stored_password, local_download_name_key, validate_remote_name_for_local_download, + ActiveTransfer, LocalDownloadStaging, }; + use std::sync::atomic::Ordering; + use std::sync::{Arc, Mutex}; + + #[tokio::test] + async fn staged_local_download_publishes_only_after_it_completed() { + let directory = tempfile::tempdir().unwrap(); + let destination = directory.path().join("existing.txt"); + tokio::fs::write(&destination, b"original").await.unwrap(); + + // A staging that never publishes keeps the destination and leaves no + // temporary behind, so a cancelled download cannot corrupt local data. + { + let mut staging = LocalDownloadStaging::begin(&destination).await.unwrap(); + staging.write(b"partial").await.unwrap(); + } + assert_eq!( + tokio::fs::read(&destination).await.unwrap(), + b"original".to_vec() + ); + assert_eq!(std::fs::read_dir(directory.path()).unwrap().count(), 1); + + let mut staging = LocalDownloadStaging::begin(&destination).await.unwrap(); + staging.write(b"complete").await.unwrap(); + staging.publish().await.unwrap(); + assert_eq!( + tokio::fs::read(&destination).await.unwrap(), + b"complete".to_vec() + ); + assert_eq!(std::fs::read_dir(directory.path()).unwrap().count(), 1); + } + + #[test] + fn active_transfer_registration_is_removed_on_every_exit_path() { + let registry: super::TransferRegistry = Arc::new(Mutex::new(Default::default())); + + { + let transfer = ActiveTransfer::register(®istry, "transfer-1").unwrap(); + let map = registry.lock().unwrap(); + assert!(map.contains_key("transfer-1")); + drop(map); + drop(transfer); + } + // `?` early returns and panics drop the guard the same way. + assert!(registry.lock().unwrap().is_empty()); + + let transfer = ActiveTransfer::register(®istry, "transfer-2").unwrap(); + assert!(!transfer.is_cancelled()); + let flag = { + let map = registry.lock().unwrap(); + map.get("transfer-2").cloned().unwrap() + }; + flag.store(true, Ordering::Relaxed); + assert!(transfer.is_cancelled()); + + // A newer transfer reusing the id keeps its own flag when an older + // guard is dropped afterwards. + let newer = ActiveTransfer::register(®istry, "transfer-2").unwrap(); + drop(transfer); + assert!(!newer.is_cancelled()); + assert!(registry.lock().unwrap().contains_key("transfer-2")); + } #[test] fn download_names_cannot_escape_the_selected_local_directory() { diff --git a/src/crates/services/services-integrations/src/remote_ssh/manager.rs b/src/crates/services/services-integrations/src/remote_ssh/manager.rs index 8d720a7946..4191053529 100644 --- a/src/crates/services/services-integrations/src/remote_ssh/manager.rs +++ b/src/crates/services/services-integrations/src/remote_ssh/manager.rs @@ -269,6 +269,27 @@ impl ContainerFileReader { } } +/// Byte source for one streamed container upload. +/// +/// Both arms feed the same staging command; only the local reader differs, so a +/// local file never has to be buffered in memory first. +enum ContainerUploadSource<'a> { + Memory(&'a [u8]), + LocalFile(std::path::PathBuf), +} + +/// Stop a container upload process and wait for its target-aware cleanup while +/// preserving the caller's error as the operation result. +async fn abort_container_upload( + control: &crate::remote_ssh::WorkspaceProcessControl, + completion: crate::remote_ssh::WorkspaceProcessCompletion, + error: anyhow::Error, +) -> anyhow::Error { + let _ = control.kill().await; + let _ = completion.wait().await; + error +} + impl AsyncRead for ContainerFileReader { fn poll_read( self: std::pin::Pin<&mut Self>, @@ -715,6 +736,22 @@ impl std::error::Error for StagedError { /// send, short enough that a chain of dead hops cannot stall shutdown. const SSH_DISCONNECT_TIMEOUT: Duration = Duration::from_secs(1); +/// Per-request SFTP response timeout. +/// +/// russh-sftp defaults to 10 s, which the 256 KiB chunk this client sends turns +/// into a ~25 KiB/s throughput floor: on a healthy but slow link every request +/// would time out instead of transferring slowly. A single stalled request is +/// still bounded here, and the SSH transport independently drops a connection +/// that stops answering. +const SFTP_REQUEST_TIMEOUT_SECS: u64 = 60; + +fn sftp_client_config() -> russh_sftp::client::Config { + russh_sftp::client::Config { + request_timeout_secs: SFTP_REQUEST_TIMEOUT_SECS, + ..Default::default() + } +} + fn tag_failed_stage(stage: &str, error: anyhow::Error) -> anyhow::Error { anyhow::Error::new(StagedError { stage: stage.to_string(), @@ -5342,6 +5379,64 @@ impl SSHConnectionManager { path: &str, content: &[u8], on_progress: &mut impl FnMut(u64, u64) -> bool, + ) -> anyhow::Result<()> { + self.container_stream_write( + connection_id, + path, + content.len() as u64, + ContainerUploadSource::Memory(content), + on_progress, + ) + .await + } + + /// Stream one local regular file into the container upload command without + /// buffering it in memory. + /// + /// The destination is replaced only after the staged bytes passed the + /// command's size check, so a cancelled or failed upload leaves the previous + /// destination untouched. + pub async fn container_write_file_from_path_with_progress( + &self, + connection_id: &str, + path: &str, + local_path: &std::path::Path, + on_progress: &mut impl FnMut(u64, u64) -> bool, + ) -> anyhow::Result { + let local_metadata = tokio::fs::symlink_metadata(local_path) + .await + .map_err(|error| { + anyhow!( + "Failed to inspect local upload file '{}': {}", + local_path.display(), + error + ) + })?; + if local_metadata.file_type().is_symlink() || !local_metadata.is_file() { + return Err(anyhow!( + "Local upload source is not a regular file: {}", + local_path.display() + )); + } + let expected_size = local_metadata.len(); + self.container_stream_write( + connection_id, + path, + expected_size, + ContainerUploadSource::LocalFile(local_path.to_path_buf()), + on_progress, + ) + .await?; + Ok(expected_size) + } + + async fn container_stream_write( + &self, + connection_id: &str, + path: &str, + expected_size: u64, + source: ContainerUploadSource<'_>, + on_progress: &mut impl FnMut(u64, u64) -> bool, ) -> anyhow::Result<()> { use tokio::io::{AsyncReadExt, AsyncWriteExt}; @@ -5359,7 +5454,6 @@ impl SSHConnectionManager { let quoted_temporary = crate::remote_ssh::shell::quote_arg(&temporary); let quoted_path = crate::remote_ssh::shell::quote_arg(&path); let quoted_parent = crate::remote_ssh::shell::quote_arg(parent); - let expected_size = content.len(); let sweep = stale_upload_sweep("ed_parent); let command = format!( "{sweep} \ @@ -5388,31 +5482,131 @@ impl SSHConnectionManager { let _ = stderr.read_to_end(&mut bytes).await; bytes }); - let total = content.len() as u64; + let total = expected_size; if !on_progress(0, total) { - let _ = control.kill().await; - let _ = completion.wait().await; - anyhow::bail!("Transfer cancelled"); + return Err(abort_container_upload( + &control, + completion, + anyhow!("Transfer cancelled"), + ) + .await); } let mut written = 0u64; - for chunk in content.chunks(256 * 1024) { - if let Err(error) = stdin.write_all(chunk).await { - let _ = control.kill().await; - let _ = completion.wait().await; - anyhow::bail!("Failed to stream container file '{}': {}", path, error); + match source { + ContainerUploadSource::Memory(content) => { + for chunk in content.chunks(256 * 1024) { + if let Err(error) = stdin.write_all(chunk).await { + return Err(abort_container_upload( + &control, + completion, + anyhow!("Failed to stream container file '{}': {}", path, error), + ) + .await); + } + written += chunk.len() as u64; + if !on_progress(written, total) { + return Err(abort_container_upload( + &control, + completion, + anyhow!("Transfer cancelled"), + ) + .await); + } + } } - written += chunk.len() as u64; - if !on_progress(written, total) { - let _ = control.kill().await; - let _ = completion.wait().await; - anyhow::bail!("Transfer cancelled"); + ContainerUploadSource::LocalFile(local_path) => { + let mut local = match tokio::fs::File::open(&local_path).await { + Ok(local) => local, + Err(error) => { + return Err(abort_container_upload( + &control, + completion, + anyhow!( + "Failed to open local upload file '{}': {}", + local_path.display(), + error + ), + ) + .await); + } + }; + let mut buffer = vec![0u8; 256 * 1024]; + loop { + let read = match local.read(&mut buffer).await { + Ok(read) => read, + Err(error) => { + return Err(abort_container_upload( + &control, + completion, + anyhow!( + "Failed to read local upload file '{}': {}", + local_path.display(), + error + ), + ) + .await); + } + }; + if read == 0 { + break; + } + if let Err(error) = stdin.write_all(&buffer[..read]).await { + return Err(abort_container_upload( + &control, + completion, + anyhow!("Failed to stream container file '{}': {}", path, error), + ) + .await); + } + written = written.saturating_add(read as u64); + if written > expected_size { + return Err(abort_container_upload( + &control, + completion, + anyhow!("Local upload file changed while it was being sent"), + ) + .await); + } + if !on_progress(written, total) { + return Err(abort_container_upload( + &control, + completion, + anyhow!("Transfer cancelled"), + ) + .await); + } + } + if written != expected_size { + return Err(abort_container_upload( + &control, + completion, + anyhow!("Local upload file changed while it was being sent"), + ) + .await); + } } } - stdin - .shutdown() - .await - .with_context(|| format!("Failed to finish container file upload '{}'", path))?; + if !on_progress(written, total) { + return Err(abort_container_upload( + &control, + completion, + anyhow!("Transfer cancelled"), + ) + .await); + } + if let Err(error) = stdin.shutdown().await { + return Err(abort_container_upload( + &control, + completion, + anyhow!( + "Failed to finish container file upload '{}': {}", + path, + error + ), + ) + .await); + } let exit = completion.wait().await; let _stdout = stdout_task.await.unwrap_or_default(); let stderr = stderr_task.await.unwrap_or_default(); @@ -5768,7 +5962,7 @@ impl SSHConnectionManager { .await .map_err(|e| anyhow!("Failed to request SFTP subsystem: {}", e))?; - let sftp = SftpSession::new(channel.into_stream()) + let sftp = SftpSession::new_with_config(channel.into_stream(), sftp_client_config()) .await .map_err(|e| anyhow!("Failed to create SFTP session: {}", e))?; @@ -5838,7 +6032,7 @@ impl SSHConnectionManager { .request_subsystem(true, "sftp") .await .map_err(|error| anyhow!("Failed to request SFTP subsystem: {}", error))?; - let session = RawSftpSession::new(channel.into_stream()); + let session = RawSftpSession::new_with_config(channel.into_stream(), sftp_client_config()); session .init() .await @@ -5973,6 +6167,34 @@ impl SSHConnectionManager { path: &str, local_path: &std::path::Path, max_bytes: u64, + ) -> anyhow::Result { + self.sftp_write_from_file_with_progress( + connection_id, + path, + local_path, + max_bytes, + &mut |_, _| true, + ) + .await + } + + /// Stream one local regular file to a remote SFTP path without buffering + /// the complete file in memory, reporting `(bytes_written, total_bytes)` + /// after every chunk. Returning `false` from `on_progress` stops the + /// transfer at the next chunk boundary. + /// + /// The destination is created before the first chunk, so a cancelled or + /// failed transfer can leave partial bytes behind. Callers that must keep + /// the previous destination intact stage the upload and publish it only + /// after this returns successfully; see + /// `RemoteFileService::write_file_from_local_path_with_progress`. + pub async fn sftp_write_from_file_with_progress( + &self, + connection_id: &str, + path: &str, + local_path: &std::path::Path, + max_bytes: u64, + on_progress: &mut impl FnMut(u64, u64) -> bool, ) -> anyhow::Result { let local_metadata = tokio::fs::symlink_metadata(local_path) .await @@ -6012,6 +6234,7 @@ impl SSHConnectionManager { use tokio::io::{AsyncReadExt, AsyncWriteExt}; let mut buffer = vec![0_u8; 256 * 1024]; let mut written = 0_u64; + let total = local_metadata.len(); loop { let read = local.read(&mut buffer).await.map_err(|error| { anyhow!( @@ -6031,6 +6254,9 @@ impl SSHConnectionManager { .write_all(&buffer[..read]) .await .map_err(|error| anyhow!("Failed to write remote file '{}': {}", path, error))?; + if !on_progress(written, total) { + return Err(anyhow!("Transfer cancelled")); + } } if written != local_metadata.len() { return Err(anyhow!("Local upload file changed while it was being sent")); @@ -6573,6 +6799,17 @@ mod tests { mod workspace_sftp; + #[test] + fn sftp_requests_tolerate_a_slow_but_healthy_link() { + let crate_default = russh_sftp::client::Config::default().request_timeout_secs; + let configured = sftp_client_config().request_timeout_secs; + assert_eq!(configured, SFTP_REQUEST_TIMEOUT_SECS); + assert!( + configured > crate_default, + "the {crate_default}s dependency default is a throughput floor for one 256 KiB chunk" + ); + } + struct UnpublishedSessionTestServer { opens: usize, delayed_open: Option, @@ -7694,6 +7931,57 @@ mod tests { assert!(!staged.exists()); } + #[cfg(unix)] + #[tokio::test] + async fn workspace_container_streams_a_local_file_and_keeps_the_destination_on_cancel() { + let temp = tempfile::tempdir().unwrap(); + let (manager, _provider) = shell_workspace_provider_fixture(temp.path()).await; + let source = temp.path().join("source file.bin"); + let target = temp.path().join("target.bin"); + let payload = vec![7_u8; 700_000]; + std::fs::write(&source, &payload).unwrap(); + std::fs::write(&target, b"preserved").unwrap(); + + let mut progress = Vec::new(); + let written = manager + .container_write_file_from_path_with_progress( + "workspace-shell-fixture", + target.to_str().unwrap(), + &source, + &mut |written, total| { + progress.push((written, total)); + true + }, + ) + .await + .unwrap(); + assert_eq!(written, payload.len() as u64); + assert_eq!( + std::fs::read(&target).unwrap(), + payload, + "a completed stream must replace the destination with the local file" + ); + assert_eq!(progress.first(), Some(&(0, payload.len() as u64))); + assert_eq!( + progress.last(), + Some(&(payload.len() as u64, payload.len() as u64)) + ); + assert!(progress.windows(2).all(|pair| pair[0].0 <= pair[1].0)); + + // The staging command publishes through a rename, so a cancelled + // stream must leave the previously committed destination untouched. + let cancelled = manager + .container_write_file_from_path_with_progress( + "workspace-shell-fixture", + target.to_str().unwrap(), + &source, + &mut |written, _| written < 262_144, + ) + .await; + assert!(cancelled.is_err()); + assert_eq!(std::fs::read(&target).unwrap(), payload); + } + #[cfg(unix)] #[tokio::test] async fn workspace_container_write_stops_after_signal_between_staging_and_commit() { diff --git a/src/crates/services/services-integrations/src/remote_ssh/manager/tests/workspace_sftp.rs b/src/crates/services/services-integrations/src/remote_ssh/manager/tests/workspace_sftp.rs index 9297092ccb..8021a04f49 100644 --- a/src/crates/services/services-integrations/src/remote_ssh/manager/tests/workspace_sftp.rs +++ b/src/crates/services/services-integrations/src/remote_ssh/manager/tests/workspace_sftp.rs @@ -400,6 +400,75 @@ async fn dependency_drop_reproduces_client_limit_with_zero_server_handles() { )); } +#[tokio::test] +async fn streamed_upload_reports_progress_and_stops_at_a_chunk_boundary() { + let f = Fixture::new().await; + let local = f._dir.path().join("upload"); + let bytes: Vec = (0..600_000).map(|index| (index % 251) as u8).collect(); + tokio::fs::write(&local, &bytes).await.unwrap(); + + let mut progress = Vec::new(); + let written = f + .manager + .sftp_write_from_file_with_progress( + "sftp-test", + "/file", + &local, + u64::MAX, + &mut |written, total| { + progress.push((written, total)); + true + }, + ) + .await + .unwrap(); + assert_eq!(written, bytes.len() as u64); + assert_eq!( + f.manager.sftp_read("sftp-test", "/file").await.unwrap(), + bytes + ); + assert_eq!( + progress.last(), + Some(&(bytes.len() as u64, bytes.len() as u64)) + ); + assert!(progress.windows(2).all(|pair| pair[0].0 <= pair[1].0)); + + let cancelled = f + .manager + .sftp_write_from_file_with_progress( + "sftp-test", + "/file", + &local, + u64::MAX, + &mut |written, _| written < 262_144, + ) + .await; + assert!(cancelled.is_err()); + assert!( + f.manager + .sftp_read("sftp-test", "/file") + .await + .unwrap() + .len() + < bytes.len(), + "a cancelled stream must stop before the whole file reaches the remote" + ); + assert_eq!(f.state.live.load(Ordering::SeqCst), 0); + + // The cancelled stream must not poison the next upload on the same channel. + assert_eq!( + f.manager + .sftp_write_from_file("sftp-test", "/file", &local, u64::MAX) + .await + .unwrap(), + bytes.len() as u64 + ); + assert_eq!( + f.manager.sftp_read("sftp-test", "/file").await.unwrap(), + bytes + ); +} + #[tokio::test] async fn repeated_manager_transfers_release_handles_and_preserve_bytes() { let f = Fixture::new().await; diff --git a/src/crates/services/services-integrations/src/remote_ssh/remote_fs.rs b/src/crates/services/services-integrations/src/remote_ssh/remote_fs.rs index 021c1b52f5..33d8a6f21d 100644 --- a/src/crates/services/services-integrations/src/remote_ssh/remote_fs.rs +++ b/src/crates/services/services-integrations/src/remote_ssh/remote_fs.rs @@ -4,6 +4,7 @@ use crate::remote_ssh::types::{RemoteDirEntry, RemoteFileEntry, RemoteTreeNode}; use anyhow::anyhow; +use openbitfun_services_core::product_identity::hidden_data_directory; use std::sync::Arc; /// Names skipped when listing workspace root for system-prompt preview (still lazy: no descent). @@ -41,6 +42,38 @@ fn remote_posix_basename(path: &str) -> String { .to_string() } +/// Sibling staging path used to publish an upload without truncating the +/// current destination. +/// +/// Remote paths are POSIX on every client platform, so the parent directory is +/// split without host `std::path` semantics. +fn staged_upload_path(destination: &str, token: &str) -> String { + let name = format!("{}-upload-{}.tmp", hidden_data_directory(), token); + match destination.rsplit_once('/') { + None => name, + Some(("", _)) => format!("/{name}"), + Some((parent, _)) => format!("{parent}/{name}"), + } +} + +/// Best-effort removal of a staged upload that never replaced its destination. +/// +/// The transfer already failed, so a failed cleanup only leaves one orphaned +/// temporary beside the destination and must not replace the real error. +async fn discard_staged_upload( + manager: &crate::remote_ssh::manager::SSHConnectionManager, + connection_id: &str, + staging: &str, +) { + if let Err(error) = manager.sftp_remove(connection_id, staging).await { + log::warn!( + "Failed to remove staged remote upload '{}': {}", + staging, + error + ); + } +} + /// Remote file service using SFTP protocol #[derive(Clone)] pub struct RemoteFileService { @@ -246,6 +279,67 @@ impl RemoteFileService { .await } + /// Stream a local file to a remote upload destination. + /// + /// The bytes land in a sibling temporary file and replace the destination + /// only after the transfer completed, so a cancelled or failed upload never + /// truncates a valid destination with partial content. The temporary name + /// follows the same shape as the container upload staging, so both + /// providers leave identically named temporaries beside the destination. + /// + /// Container workspaces run their own staging command, which verifies the + /// received size and publishes the destination itself, so they stream + /// straight into that command instead of through SFTP. + pub async fn write_file_from_local_path_with_progress( + &self, + connection_id: &str, + path: &str, + local_path: &std::path::Path, + on_progress: &mut impl FnMut(u64, u64) -> bool, + ) -> anyhow::Result { + let manager = self.get_manager(connection_id).await?; + if manager.is_shell_workspace(connection_id).await { + return manager + .container_write_file_from_path_with_progress( + connection_id, + path, + local_path, + on_progress, + ) + .await; + } + + let staging = staged_upload_path(path, &uuid::Uuid::new_v4().to_string()); + let written = match manager + .sftp_write_from_file_with_progress( + connection_id, + &staging, + local_path, + u64::MAX, + on_progress, + ) + .await + { + Ok(written) => written, + Err(error) => { + discard_staged_upload(&manager, connection_id, &staging).await; + return Err(error); + } + }; + // An empty file has no chunk callback, and a stop can also arrive + // while the final chunk is being flushed. Check once more before the + // staged file becomes the destination. + if !on_progress(written, written) { + discard_staged_upload(&manager, connection_id, &staging).await; + anyhow::bail!("Transfer cancelled"); + } + if let Err(error) = self.atomic_replace(connection_id, &staging, path).await { + discard_staged_upload(&manager, connection_id, &staging).await; + return Err(error); + } + Ok(written) + } + /// Check if a remote path exists pub async fn exists(&self, connection_id: &str, path: &str) -> anyhow::Result { let manager = self.get_manager(connection_id).await?; @@ -692,11 +786,35 @@ fn format_permissions(mode: Option) -> String { #[cfg(test)] mod tests { use super::{ - remote_file_entry_from_metadata, remote_file_entry_from_stat_result, remote_posix_basename, + hidden_data_directory, remote_file_entry_from_metadata, remote_file_entry_from_stat_result, + remote_posix_basename, staged_upload_path, }; use russh_sftp::client::error::Error as SftpError; use russh_sftp::protocol::{Status, StatusCode}; + #[test] + fn staged_upload_stays_beside_its_destination_directory() { + let hidden = hidden_data_directory(); + assert_eq!( + staged_upload_path("/workspace/目录/name.txt", "token"), + format!("/workspace/目录/{hidden}-upload-token.tmp") + ); + assert_eq!( + staged_upload_path("relative/name.txt", "token"), + format!("relative/{hidden}-upload-token.tmp") + ); + // A workspace-root and a bare filename must not gain a parent the + // destination never had, or the publish would move across directories. + assert_eq!( + staged_upload_path("/name.txt", "token"), + format!("/{hidden}-upload-token.tmp") + ); + assert_eq!( + staged_upload_path("name.txt", "token"), + format!("{hidden}-upload-token.tmp") + ); + } + #[test] fn remote_basename_never_uses_host_path_separators() { assert_eq!( diff --git a/src/web-ui/src/app/components/panels/FilesPanel.tsx b/src/web-ui/src/app/components/panels/FilesPanel.tsx index c2dca4502d..a03e0fd98d 100644 --- a/src/web-ui/src/app/components/panels/FilesPanel.tsx +++ b/src/web-ui/src/app/components/panels/FilesPanel.tsx @@ -46,6 +46,7 @@ import type { WorkspaceSearchRepoPhase, } from '@/infrastructure/api/service-api/tauri-commands'; import { + cancelWorkspaceTransfer, downloadWorkspaceFileToDisk, joinWorkspaceTargetPath, normalizeWorkspaceTargetDirectory, @@ -187,7 +188,6 @@ const FilesPanel: React.FC = ({ const [renamingPath, setRenamingPath] = useState(null); const [transfers, setTransfers] = useState>(new Map()); - const dropTransferIdRef = useRef(null); const [fileDropHighlight, setFileDropHighlight] = useState(false); const [inputDialog, setInputDialog] = useState<{ isOpen: boolean; @@ -211,7 +211,11 @@ const FilesPanel: React.FC = ({ const createTransferProgress = useCallback(() => { const id = crypto.randomUUID(); const onProgress = (state: TransferProgressState | null) => { + const wasStopped = state !== null && cancelledTransferIdsRef.current.has(id); setTransfers((prev) => { + if (wasStopped) { + return prev; + } const next = new Map(prev); if (state === null) { next.delete(id); @@ -220,6 +224,9 @@ const FilesPanel: React.FC = ({ } return next; }); + if (state === null) { + cancelledTransferIdsRef.current.delete(id); + } }; return { id, onProgress }; }, []); @@ -227,6 +234,9 @@ const FilesPanel: React.FC = ({ /** Stop an in-progress transfer by its ID. */ const handleStopTransfer = useCallback((transferId: string) => { cancelledTransferIdsRef.current.add(transferId); + // A peer download runs in this process, so it also needs the stop signal + // that `cancel_transfer` can only deliver to the host running the transfer. + cancelWorkspaceTransfer(transferId); void sshApi.cancelTransfer(transferId); setTransfers((prev) => { const next = new Map(prev); @@ -236,27 +246,27 @@ const FilesPanel: React.FC = ({ }, []); /** - * Stable callback for drag-and-drop file uploads. Uses a ref to track the - * current drop's transfer ID so each drop session gets its own entry in the - * `transfers` Map. + * Stable callback for drag-and-drop file uploads. The drop hook owns the + * transfer id and reports it with every progress state, so the card and the + * cancellable backend transfer stay the same operation. */ - const handleDropProgress = useCallback((state: TransferProgressState | null) => { + const handleDropProgress = useCallback((state: TransferProgressState | null, transferId: string) => { + const wasStopped = state !== null && cancelledTransferIdsRef.current.has(transferId); setTransfers((prev) => { + if (wasStopped) { + return prev; + } const next = new Map(prev); if (state === null) { - const id = dropTransferIdRef.current; - if (id) { - next.delete(id); - dropTransferIdRef.current = null; - } + next.delete(transferId); } else { - if (!dropTransferIdRef.current) { - dropTransferIdRef.current = crypto.randomUUID(); - } - next.set(dropTransferIdRef.current, state); + next.set(transferId, state); } return next; }); + if (state === null) { + cancelledTransferIdsRef.current.delete(transferId); + } }, []); const searchLimitNotice = @@ -515,10 +525,9 @@ const FilesPanel: React.FC = ({ ); } catch (error) { log.error('Failed to download file', error); + const wasStopped = cancelledTransferIdsRef.current.has(id); onProgress(null); - if (cancelledTransferIdsRef.current.has(id)) { - cancelledTransferIdsRef.current.delete(id); - } else { + if (!wasStopped) { notification.error(t('transfer.failed', { error: String(error) })); } } @@ -671,6 +680,13 @@ const FilesPanel: React.FC = ({ id ); + if (result.cancelled && result.failedFiles.length === 0) { + if (result.successCount > 0) { + await loadFileTree(undefined, true); + } + return; + } + if (result.successCount === 0 && result.failedFiles.length === 0) { notification.info(t('notifications.pasteNoFiles')); return; @@ -706,10 +722,9 @@ const FilesPanel: React.FC = ({ } } catch (error) { log.error('Failed to paste files', error); + const wasStopped = cancelledTransferIdsRef.current.has(id); onProgress(null); - if (cancelledTransferIdsRef.current.has(id)) { - cancelledTransferIdsRef.current.delete(id); - } else { + if (!wasStopped) { notification.error(t('notifications.pasteFailed', { count: 1 })); } } @@ -892,10 +907,16 @@ const FilesPanel: React.FC = ({ } }, [workspacePath, loadFileTree, expandFolder]); - const handleFileDropError = useCallback((error: unknown) => { - handleDropProgress(null); + const handleFileDropError = useCallback(( + error: unknown, + transferId: string, + ) => { + const wasStopped = cancelledTransferIdsRef.current.has(transferId); + handleDropProgress(null, transferId); setFileDropHighlight(false); - notification.error(t('transfer.failed', { error: String(error) })); + if (!wasStopped) { + notification.error(t('transfer.failed', { error: String(error) })); + } }, [notification, t, handleDropProgress]); useWorkspaceFileDrop({ diff --git a/src/web-ui/src/features/ssh-remote/RemoteFileBrowser.test.tsx b/src/web-ui/src/features/ssh-remote/RemoteFileBrowser.test.tsx index ef2a14f230..71f95ed437 100644 --- a/src/web-ui/src/features/ssh-remote/RemoteFileBrowser.test.tsx +++ b/src/web-ui/src/features/ssh-remote/RemoteFileBrowser.test.tsx @@ -10,9 +10,12 @@ globalThis.IS_REACT_ACT_ENVIRONMENT = true; const api = vi.hoisted(() => ({ readDir: vi.fn(async () => [{ name: 'old.txt', path: '/srv/project/old.txt', isDir: false }]), rename: vi.fn(async () => undefined), + downloadToLocalPath: vi.fn(), + cancelTransfer: vi.fn(async () => undefined), })); vi.mock('./sshApi', () => ({ sshApi: api })); vi.mock('@/infrastructure/i18n', () => ({ useI18n: () => ({ t: (key: string) => key }) })); +vi.mock('@tauri-apps/plugin-dialog', () => ({ save: vi.fn(async () => '/downloads/old.txt') })); it('renames a remote file once and keeps IME and outside clicks from dismissing the dialog', async () => { const container = document.createElement('div'); @@ -50,3 +53,33 @@ it('renames a remote file once and keeps IME and outside clicks from dismissing container.remove(); } }); + +it('stops a browser download using the id of its in-flight command', async () => { + Object.defineProperty(window, '__TAURI__', { value: {}, configurable: true }); + let finishDownload!: () => void; + api.downloadToLocalPath.mockImplementationOnce(() => new Promise((resolve) => { + finishDownload = resolve; + })); + const container = document.createElement('div'); + document.body.appendChild(container); + const root = createRoot(container); + try { + await act(async () => root.render( undefined} onCancel={() => undefined} />)); + const row = [...document.querySelectorAll('.remote-file-browser__row')].find(node => node.textContent?.includes('old.txt'))!; + act(() => row.dispatchEvent(new MouseEvent('contextmenu', { bubbles: true }))); + const download = [...document.querySelectorAll('[role="menuitem"]')].find(node => node.textContent === 'ssh.remote.download')!; + await act(async () => download.click()); + + const transferId = api.downloadToLocalPath.mock.calls[0]?.[4]; + expect(transferId).toEqual(expect.any(String)); + const stop = [...document.querySelectorAll('button')].find(node => node.textContent === 'actions.cancel')!; + await act(async () => stop.click()); + expect(api.cancelTransfer).toHaveBeenCalledExactlyOnceWith(transferId); + expect(api.downloadToLocalPath.mock.calls[0]?.[5]()).toBe(true); + await act(async () => finishDownload()); + } finally { + act(() => root.unmount()); + container.remove(); + delete (window as Window & { __TAURI__?: unknown }).__TAURI__; + } +}); diff --git a/src/web-ui/src/features/ssh-remote/RemoteFileBrowser.tsx b/src/web-ui/src/features/ssh-remote/RemoteFileBrowser.tsx index 6a039a558a..2608866404 100644 --- a/src/web-ui/src/features/ssh-remote/RemoteFileBrowser.tsx +++ b/src/web-ui/src/features/ssh-remote/RemoteFileBrowser.tsx @@ -37,6 +37,11 @@ interface DeleteConfirmState { entry: RemoteFileEntry | null; } +interface ActiveBrowserTransfer { + id: string; + cancelled: boolean; +} + function joinRemotePath(dir: string, fileName: string): string { const name = fileName.replace(/^\/+/, ''); if (!dir || dir === '/') { @@ -104,8 +109,44 @@ export const RemoteFileBrowser: React.FC = ({ entry: null, }); const [transferBusy, setTransferBusy] = useState(false); + const [transferCancelling, setTransferCancelling] = useState(false); + const activeTransferRef = useRef(null); const contextMenuRef = useRef(null); + const beginTransfer = (): ActiveBrowserTransfer | null => { + if (activeTransferRef.current) return null; + const transfer = { id: crypto.randomUUID(), cancelled: false }; + activeTransferRef.current = transfer; + setTransferBusy(true); + setTransferCancelling(false); + setError(null); + return transfer; + }; + + const endTransfer = (transfer: ActiveBrowserTransfer): void => { + if (activeTransferRef.current !== transfer) return; + activeTransferRef.current = null; + setTransferBusy(false); + setTransferCancelling(false); + }; + + const stopTransfer = (): void => { + const transfer = activeTransferRef.current; + if (!transfer || transfer.cancelled) return; + transfer.cancelled = true; + setTransferCancelling(true); + void sshApi.cancelTransfer(transfer.id).catch((error: unknown) => { + setError(error instanceof Error ? error.message : t('ssh.remote.transferFailed')); + }); + }; + + useEffect(() => () => { + const transfer = activeTransferRef.current; + if (!transfer) return; + transfer.cancelled = true; + void sshApi.cancelTransfer(transfer.id).catch(() => undefined); + }, []); + // One-shot retry: when the SSH session was torn down by a transient network // blip, the backend transparently reconnects on the next call but the // already-in-flight request still fails. Retrying once gives the recovery @@ -230,14 +271,19 @@ export const RemoteFileBrowser: React.FC = ({ }); if (localPath === null) return; - setTransferBusy(true); - setError(null); + const transfer = beginTransfer(); + if (!transfer) return; try { - await sshApi.downloadToLocalPath(connectionId, entry.path, localPath); + await sshApi.downloadToLocalPath( + connectionId, entry.path, localPath, undefined, transfer.id, + () => transfer.cancelled, + ); } catch (e) { - setError(e instanceof Error ? e.message : t('ssh.remote.transferFailed')); + if (!transfer.cancelled) { + setError(e instanceof Error ? e.message : t('ssh.remote.transferFailed')); + } } finally { - setTransferBusy(false); + endTransfer(transfer); } }; @@ -322,21 +368,29 @@ export const RemoteFileBrowser: React.FC = ({ const paths = Array.isArray(selected) ? selected : [selected]; if (paths.length === 0) return; - setTransferBusy(true); - setError(null); + const transfer = beginTransfer(); + if (!transfer) return; try { for (const localPath of paths) { + if (transfer.cancelled) break; const segments = localPath.split(/[/\\]/); const base = segments.pop(); if (!base) continue; const remotePath = joinRemotePath(currentPath, base); - await sshApi.uploadFromLocalPath(connectionId, localPath, remotePath); + await sshApi.uploadFromLocalPath( + connectionId, localPath, remotePath, undefined, transfer.id, + () => transfer.cancelled, + ); } await loadDirectory(currentPath); } catch (e) { - setError(e instanceof Error ? e.message : t('ssh.remote.transferFailed')); + if (!transfer.cancelled) { + setError(e instanceof Error ? e.message : t('ssh.remote.transferFailed')); + } else { + void loadDirectory(currentPath); + } } finally { - setTransferBusy(false); + endTransfer(transfer); } }; @@ -506,6 +560,15 @@ export const RemoteFileBrowser: React.FC = ({
{t('ssh.remote.transferring')} +
)} diff --git a/src/web-ui/src/features/ssh-remote/sshApi.test.ts b/src/web-ui/src/features/ssh-remote/sshApi.test.ts new file mode 100644 index 0000000000..38b7d26f09 --- /dev/null +++ b/src/web-ui/src/features/ssh-remote/sshApi.test.ts @@ -0,0 +1,63 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const invoke = vi.hoisted(() => vi.fn()); + +vi.mock('@/infrastructure/api/service-api/ApiClient', () => ({ + api: { invoke }, +})); + +import { sshApi } from './sshApi'; + +describe('sshApi request deadlines', () => { + beforeEach(() => { + invoke.mockReset(); + invoke.mockResolvedValue(undefined); + }); + + it('lets a file transfer run past the default request deadline', async () => { + await sshApi.downloadToLocalPath('connection-1', '/remote/a.bin', '/local/a.bin', undefined, 'download-1'); + expect(invoke).toHaveBeenCalledWith( + 'remote_download_to_local_path', + expect.objectContaining({ transferId: expect.any(String) }), + { timeout: 0 }, + ); + + await sshApi.uploadFromLocalPath('connection-1', '/local/b.bin', '/remote/b.bin', undefined, 'upload-1'); + expect(invoke).toHaveBeenLastCalledWith( + 'remote_upload_from_local_path', + expect.objectContaining({ transferId: expect.any(String) }), + { timeout: 0 }, + ); + }); + + it('keeps the default deadline for commands with no stop action', async () => { + await sshApi.readFile('connection-1', '/remote/a.txt'); + // No third argument: the shared client keeps its own deadline, which is + // what ends a command whose host never answers. + expect(invoke).toHaveBeenCalledWith('remote_read_file', { + connectionId: 'connection-1', + path: '/remote/a.txt', + }); + + await sshApi.downloadToLocalPath('connection-1', '/remote/a.bin', '/local/a.bin'); + expect(invoke).toHaveBeenLastCalledWith( + 'remote_download_to_local_path', + expect.objectContaining({ transferId: expect.any(String) }), + undefined, + ); + + await sshApi.uploadFromLocalPath('connection-1', '/local/b.bin', '/remote/b.bin'); + expect(invoke).toHaveBeenLastCalledWith( + 'remote_upload_from_local_path', + expect.objectContaining({ transferId: expect.any(String) }), + undefined, + ); + }); + + it('does not start a transfer stopped before the backend registration', async () => { + await expect(sshApi.uploadFromLocalPath( + 'connection-1', '/local/b.bin', '/remote/b.bin', undefined, 'stopped', () => true, + )).rejects.toThrow('Transfer cancelled'); + expect(invoke).not.toHaveBeenCalled(); + }); +}); diff --git a/src/web-ui/src/features/ssh-remote/sshApi.ts b/src/web-ui/src/features/ssh-remote/sshApi.ts index 4407c2df12..b5e1c205f2 100644 --- a/src/web-ui/src/features/ssh-remote/sshApi.ts +++ b/src/web-ui/src/features/ssh-remote/sshApi.ts @@ -23,6 +23,20 @@ import type { // API adapter for Tauri/Server Mode compatibility import { api } from '@/infrastructure/api/service-api/ApiClient'; +/** + * Deadline override for the remote file transfers that expose a stop action. + * + * These commands are already bounded where the work happens: every SFTP request + * has its own response timeout, the SSH transport drops a stalled connection, + * and the transfer itself can be stopped from its card. A wall-clock deadline + * here only guaranteed failure for slow-but-healthy links, which is how a + * completed transfer used to surface as "Request timeout". + * + * Commands without a progress or stop surface keep the default deadline: it is + * what keeps an unresponsive host from freezing the UI with no way out. + */ +const NO_REQUEST_DEADLINE = { timeout: 0 } as const; + export const sshApi = { // === Connection Management === @@ -212,6 +226,7 @@ export const sshApi = { localPath: string, onProgress?: (downloaded: number, total: number) => void, transferId?: string, + isCancelled?: () => boolean, ): Promise { const tid = transferId ?? crypto.randomUUID(); let unlisten: (() => void) | null = null; @@ -229,12 +244,17 @@ export const sshApi = { } try { + // The stop action can run while the progress listener is being installed, + // before the backend has registered this transfer id. + if (isCancelled?.()) { + throw new Error('Transfer cancelled'); + } await api.invoke('remote_download_to_local_path', { connectionId, remotePath, localPath, transferId: tid, - }); + }, transferId ? NO_REQUEST_DEADLINE : undefined); } finally { unlisten?.(); } @@ -258,6 +278,7 @@ export const sshApi = { remotePath: string, onProgress?: (uploaded: number, total: number) => void, transferId?: string, + isCancelled?: () => boolean, ): Promise<{ wasDirectory: boolean }> { const tid = transferId ?? crypto.randomUUID(); let unlisten: (() => void) | null = null; @@ -275,12 +296,15 @@ export const sshApi = { } try { + if (isCancelled?.()) { + throw new Error('Transfer cancelled'); + } return await api.invoke('remote_upload_from_local_path', { connectionId, localPath, remotePath, transferId: tid, - }); + }, transferId ? NO_REQUEST_DEADLINE : undefined); } finally { unlisten?.(); } diff --git a/src/web-ui/src/infrastructure/api/service-api/ApiClient.test.ts b/src/web-ui/src/infrastructure/api/service-api/ApiClient.test.ts index 2963f28d32..099278093c 100644 --- a/src/web-ui/src/infrastructure/api/service-api/ApiClient.test.ts +++ b/src/web-ui/src/infrastructure/api/service-api/ApiClient.test.ts @@ -245,6 +245,38 @@ describe('ApiClient startup trace classification', () => { } }); + it('lets a streamed transfer opt out of the request deadline', async () => { + vi.useFakeTimers(); + try { + let settle: (value: unknown) => void = () => {}; + adapterMocks.request.mockReturnValueOnce( + new Promise((resolve) => { + settle = resolve; + }), + ); + const client = new ApiClient({ enableLogging: false, retries: 0 }); + + const pending = client.invoke( + 'remote_download_to_local_path', + {}, + { timeout: 0 }, + ); + // Any wall-clock budget is longer than the default 30s deadline, so a + // transfer that is still running must not be aborted. + await vi.advanceTimersByTimeAsync(24 * 60 * 60 * 1000); + settle('transferred'); + + await expect(pending).resolves.toBe('transferred'); + expect(client.getStats()).toMatchObject({ + successfulRequests: 1, + failedRequests: 0, + activeRequests: 0, + }); + } finally { + vi.useRealTimers(); + } + }); + it('uses the message from plain structured Tauri errors', async () => { const transportError = { code: 'worktree_not_found', diff --git a/src/web-ui/src/infrastructure/api/service-api/ApiClient.ts b/src/web-ui/src/infrastructure/api/service-api/ApiClient.ts index 1b8b318b61..e5d512b557 100644 --- a/src/web-ui/src/infrastructure/api/service-api/ApiClient.ts +++ b/src/web-ui/src/infrastructure/api/service-api/ApiClient.ts @@ -355,9 +355,14 @@ export class ApiClient implements IApiClient { }); - const timeoutId = setTimeout(() => { - controller.abort(); - }, request.config.timeout || this.config.timeout); + // A non-positive timeout opts out of the deadline. `??` is deliberate: a + // caller that disables the deadline must not fall back to the default. + const timeoutMs = request.config.timeout ?? this.config.timeout; + const timeoutId = timeoutMs > 0 + ? setTimeout(() => { + controller.abort(); + }, timeoutMs) + : undefined; try { @@ -379,7 +384,9 @@ export class ApiClient implements IApiClient { }); this.assertRequestSurface(request, traceCommand); - clearTimeout(timeoutId); + if (timeoutId !== undefined) { + clearTimeout(timeoutId); + } maxConcurrentRequests = this.activeRequestPressure.get(request.id)?.maxConcurrentRequests ?? this.activeRequests.size; this.activeRequests.delete(request.id); this.activeRequestPressure.delete(request.id); @@ -439,7 +446,9 @@ export class ApiClient implements IApiClient { maxConcurrentRequests = maxConcurrentRequests || this.activeRequestPressure.get(request.id)?.maxConcurrentRequests || this.activeRequests.size; - clearTimeout(timeoutId); + if (timeoutId !== undefined) { + clearTimeout(timeoutId); + } this.activeRequests.delete(request.id); this.activeRequestPressure.delete(request.id); activeRequestsAtEnd = this.activeRequests.size; diff --git a/src/web-ui/src/infrastructure/api/service-api/types.ts b/src/web-ui/src/infrastructure/api/service-api/types.ts index 8aec338b23..cd0e689595 100644 --- a/src/web-ui/src/infrastructure/api/service-api/types.ts +++ b/src/web-ui/src/infrastructure/api/service-api/types.ts @@ -19,6 +19,14 @@ export interface ApiError { export interface ApiRequestConfig { + /** + * Deadline for this request in milliseconds. + * + * Non-positive values disable the deadline, which long-running streaming + * commands (file transfers) use because their own layer already bounds every + * protocol request and the user can stop them explicitly. Such a command must + * provide cancellation; without a deadline nothing else ends the wait. + */ timeout?: number; retries?: number; retryDelay?: number; diff --git a/src/web-ui/src/locales/en-US/panels/files.json b/src/web-ui/src/locales/en-US/panels/files.json index 685e41a5cb..84e3cb35af 100644 --- a/src/web-ui/src/locales/en-US/panels/files.json +++ b/src/web-ui/src/locales/en-US/panels/files.json @@ -128,6 +128,7 @@ "downloading": "Downloading", "uploading": "Uploading", "missingConnection": "Remote workspace has no SSH connection id.", + "cancelled": "Transfer cancelled", "failed": "File transfer failed: {{error}}", "dropHint": "Drop files here to upload", "stop": "Stop" diff --git a/src/web-ui/src/locales/zh-CN/panels/files.json b/src/web-ui/src/locales/zh-CN/panels/files.json index 75826d53ca..369e9a45a4 100644 --- a/src/web-ui/src/locales/zh-CN/panels/files.json +++ b/src/web-ui/src/locales/zh-CN/panels/files.json @@ -128,6 +128,7 @@ "downloading": "正在下载", "uploading": "正在上传", "missingConnection": "远程工作区缺少 SSH 连接信息。", + "cancelled": "传输已取消", "failed": "文件传输失败:{{error}}", "dropHint": "将文件拖放到此处以上传", "stop": "停止" diff --git a/src/web-ui/src/locales/zh-TW/panels/files.json b/src/web-ui/src/locales/zh-TW/panels/files.json index 4b38fa84fb..8395e60b0c 100644 --- a/src/web-ui/src/locales/zh-TW/panels/files.json +++ b/src/web-ui/src/locales/zh-TW/panels/files.json @@ -128,6 +128,7 @@ "downloading": "正在下載", "uploading": "正在上傳", "missingConnection": "遠程工作區缺少 SSH 連接資訊。", + "cancelled": "傳輸已取消", "failed": "檔案傳輸失敗:{{error}}", "dropHint": "將檔案拖放到此處以上傳", "stop": "停止" diff --git a/src/web-ui/src/tools/file-system/hooks/useWorkspaceFileDrop.ts b/src/web-ui/src/tools/file-system/hooks/useWorkspaceFileDrop.ts index 7df09912b4..f15989f6b3 100644 --- a/src/web-ui/src/tools/file-system/hooks/useWorkspaceFileDrop.ts +++ b/src/web-ui/src/tools/file-system/hooks/useWorkspaceFileDrop.ts @@ -17,10 +17,10 @@ export interface UseWorkspaceFileDropOptions { workspacePath?: string; panelRef: RefObject; enabled?: boolean; - onProgress: (state: TransferProgressState | null) => void; + onProgress: (state: TransferProgressState | null, transferId: string) => void; onDragOver?: (overPanel: boolean) => void; - onComplete: (targetDirectory: string) => void; - onError: (error: unknown) => void; + onComplete: (targetDirectory: string, transferId: string) => void; + onError: (error: unknown, transferId: string) => void; } export function useWorkspaceFileDrop({ @@ -151,17 +151,23 @@ export function useWorkspaceFileDrop({ isDragOverPanelRef.current = false; dropProcessingRef.current = true; + // One id per drop session: it keys the progress card and reaches the + // backend, so the card's stop action cancels this upload instead of + // sending an id no transfer ever registered. + const transferId = crypto.randomUUID(); try { await uploadLocalPathsToWorkspaceDirectory( paths, targetDir, currentWorkspace, - onProgress + (state) => onProgress(state, transferId), + undefined, + transferId ); - onComplete(targetDir); + onComplete(targetDir, transferId); } catch (error) { log.error('Failed to upload dropped files', error); - onError(error); + onError(error, transferId); } finally { dropProcessingRef.current = false; } diff --git a/src/web-ui/src/tools/file-system/services/workspaceFileTransfer.test.ts b/src/web-ui/src/tools/file-system/services/workspaceFileTransfer.test.ts index fcb66d01af..e176f44d14 100644 --- a/src/web-ui/src/tools/file-system/services/workspaceFileTransfer.test.ts +++ b/src/web-ui/src/tools/file-system/services/workspaceFileTransfer.test.ts @@ -1,5 +1,6 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { + cancelWorkspaceTransfer, decodeBase64FileChunk, writeAllToLocalFile, readPeerFileChunks, @@ -7,7 +8,10 @@ import { joinWorkspaceTargetPath, normalizeClipboardLocalPaths, resolvePasteTargetDirectory, + uploadLocalPathsToWorkspaceDirectory, } from "./workspaceFileTransfer"; +import { WorkspaceKind, WorkspaceType, type WorkspaceInfo } from "@/shared/types"; +import { i18nService } from "@/infrastructure/i18n"; describe("workspaceFileTransfer", () => { it("decodes peer file chunks without corrupting binary bytes", () => { @@ -157,4 +161,180 @@ describe("fixed peer download identity", () => { await stream.next(); await expect(stream.next()).rejects.toThrow("changed during download"); }); + + it("stops a peer download when the controller-side stop request arrives", async () => { + const requestPeerCommand = vi.fn() + .mockResolvedValueOnce({ resp: "file_info", size: 4 }) + .mockImplementationOnce(async () => { + // The user pressed stop while the first chunk was in flight. + cancelWorkspaceTransfer("peer-transfer"); + return { resp: "file_chunk", offset: 0, chunk_size: 1, total_size: 4, chunk_base64: "AQ==", revision: "r1" }; + }); + const adapter = { requestPeerCommand } as unknown as Parameters[0]; + const stream = readPeerFileChunks( + adapter, + "/workspace/file", + vi.fn(), + { workspace_path: "/workspace" }, + "peer-transfer", + ); + expect((await stream.next()).value).toEqual(new Uint8Array([1])); + await expect(stream.next()).rejects.toThrow( + i18nService.t("panels/files:transfer.cancelled"), + ); + expect(requestPeerCommand).toHaveBeenCalledTimes(2); + + // The id is only stopped once: a later chunk read for another transfer of + // the same file must proceed. + const retry = vi.fn() + .mockResolvedValueOnce({ resp: "file_info", size: 1 }) + .mockResolvedValueOnce({ resp: "file_chunk", offset: 0, chunk_size: 1, total_size: 1, chunk_base64: "AQ==", revision: "r1" }); + const retryAdapter = { requestPeerCommand: retry } as unknown as Parameters[0]; + const chunks: number[] = []; + for await (const chunk of readPeerFileChunks( + retryAdapter, + "/workspace/file", + vi.fn(), + { workspace_path: "/workspace" }, + "another-transfer", + )) { + chunks.push(...chunk); + } + expect(chunks).toEqual([1]); + }); + + it("does not publish a peer file stopped after its final chunk", async () => { + const requestPeerCommand = vi.fn() + .mockResolvedValueOnce({ resp: "file_info", size: 1 }) + .mockResolvedValueOnce({ resp: "file_chunk", offset: 0, chunk_size: 1, total_size: 1, chunk_base64: "AQ==", revision: "r1" }); + const adapter = { requestPeerCommand } as unknown as Parameters[0]; + const stream = readPeerFileChunks( + adapter, "/workspace/file", vi.fn(), { workspace_path: "/workspace" }, "last-chunk", + ); + expect((await stream.next()).value).toEqual(new Uint8Array([1])); + cancelWorkspaceTransfer("last-chunk"); + await expect(stream.next()).rejects.toThrow( + i18nService.t("panels/files:transfer.cancelled"), + ); + }); +}); + +describe("remote workspace uploads report their cancellable transfer id", () => { + const uploadFromLocalPath = vi.fn(); + + beforeEach(() => { + vi.resetModules(); + vi.resetAllMocks(); + uploadFromLocalPath.mockResolvedValue({ wasDirectory: false }); + (globalThis as { window?: unknown }).window = globalThis; + (globalThis as unknown as { __TAURI__: unknown }).__TAURI__ = {}; + }); + + const remoteWorkspace: WorkspaceInfo = { + id: "workspace-1", + name: "remote", + rootPath: "/workspace", + workspaceType: WorkspaceType.SingleProject, + workspaceKind: WorkspaceKind.Remote, + connectionId: "saved-ssh", + languages: [], + openedAt: "2024-01-01T00:00:00.000Z", + lastAccessed: "2024-01-01T00:00:00.000Z", + tags: [], + }; + + async function loadTransferModule() { + vi.doMock("@/features/ssh-remote/sshApi", () => ({ + sshApi: { uploadFromLocalPath }, + })); + return await import("./workspaceFileTransfer"); + } + + it("passes one id to the backend command and to every progress state", async () => { + const { uploadLocalPathsToWorkspaceDirectory: upload } = + await loadTransferModule(); + const states: Array<{ transferId?: string }> = []; + const result = await upload( + ["/local/a.txt"], + "/workspace", + remoteWorkspace, + (state) => states.push(state ?? {}), + undefined, + "transfer-42", + ); + + expect(result.successCount).toBe(1); + expect(uploadFromLocalPath).toHaveBeenCalledTimes(1); + const [, , , , forwardedId] = uploadFromLocalPath.mock.calls[0]!; + expect(forwardedId).toBe("transfer-42"); + expect(states.filter((state) => state.transferId).length).toBeGreaterThan(0); + for (const state of states) { + if (state.transferId) { + expect(state.transferId).toBe("transfer-42"); + } + } + }); + + it("stops the remaining items of a multi-file upload after a stop request", async () => { + const { cancelWorkspaceTransfer, uploadLocalPathsToWorkspaceDirectory: upload } = + await loadTransferModule(); + // The first item is stopped while it is being sent. The remaining items + // must not start, and a user stop is not reported as a failed file. + uploadFromLocalPath.mockImplementation(async () => { + cancelWorkspaceTransfer("transfer-multi"); + throw new Error("Transfer cancelled"); + }); + const states: Array<{ transferId?: string }> = []; + + const result = await upload( + ["/local/a.txt", "/local/b.txt", "/local/c.txt"], + "/workspace", + remoteWorkspace, + (state) => states.push(state ?? {}), + undefined, + "transfer-multi", + ); + expect(result).toMatchObject({ successCount: 0, failedFiles: [], cancelled: true }); + expect(uploadFromLocalPath).toHaveBeenCalledTimes(1); + }); + + it("keeps completed items without treating a later stop as a failure", async () => { + const { cancelWorkspaceTransfer, uploadLocalPathsToWorkspaceDirectory: upload } = + await loadTransferModule(); + uploadFromLocalPath + .mockResolvedValueOnce({ wasDirectory: false }) + .mockImplementationOnce(async () => { + cancelWorkspaceTransfer("transfer-partial"); + throw new Error("Transfer cancelled"); + }); + + const result = await upload( + ["/local/a.txt", "/local/b.txt", "/local/c.txt"], + "/workspace", + remoteWorkspace, + () => undefined, + undefined, + "transfer-partial", + ); + expect(result).toMatchObject({ successCount: 1, failedFiles: [], cancelled: true }); + expect(uploadFromLocalPath).toHaveBeenCalledTimes(2); + }); + + it("does not start an upload stopped from its first progress card", async () => { + const { cancelWorkspaceTransfer, uploadLocalPathsToWorkspaceDirectory: upload } = + await loadTransferModule(); + const result = await upload( + ["/local/a.txt"], + "/workspace", + remoteWorkspace, + (state) => { + if (state?.current === 0) cancelWorkspaceTransfer("before-start"); + }, + undefined, + "before-start", + ); + expect(result.successCount).toBe(0); + expect(result.cancelled).toBe(true); + expect(uploadFromLocalPath).not.toHaveBeenCalled(); + }); }); diff --git a/src/web-ui/src/tools/file-system/services/workspaceFileTransfer.ts b/src/web-ui/src/tools/file-system/services/workspaceFileTransfer.ts index a6551a65c4..04298a1645 100644 --- a/src/web-ui/src/tools/file-system/services/workspaceFileTransfer.ts +++ b/src/web-ui/src/tools/file-system/services/workspaceFileTransfer.ts @@ -35,12 +35,21 @@ export interface TransferProgressState { bytesTotal?: number; /** Transfer speed in bytes per second (smoothed) */ speed?: number; + /** + * Transfer id this progress belongs to. + * + * A path that can be stopped must report the same id it registered, so a + * progress card's stop action reaches the operation it displays. + */ + transferId?: string; } export interface WorkspaceTransferResult { successCount: number; directoryCount: number; failedFiles: Array<{ path: string; error: string }>; + /** A user stopped the remaining items; completed items still count. */ + cancelled?: boolean; } export interface UploadToWorkspaceOptions { @@ -99,6 +108,45 @@ function currentPeerAdapter(): PeerDeviceTransportAdapter | null { return adapter instanceof PeerDeviceTransportAdapter ? adapter : null; } +/** + * Controller-side stop requests for transfers the controller executes itself. + * + * A peer download reads chunks through this process, so its stop signal has to + * live here: `cancel_transfer` only reaches the host that runs the SSH + * connection, and in Peer Device Mode no such command is in flight. + */ +const cancelledWorkspaceTransfers = new Set(); + +/** + * Stop a transfer started by this module. Returns whether an id was stopped + * locally; the remote host still has to be told separately for hosted + * transfers. + */ +export function cancelWorkspaceTransfer(transferId: string): boolean { + if (!transferId) { + return false; + } + cancelledWorkspaceTransfers.add(transferId); + return true; +} + +/** Unregister a finished transfer so its stop mark cannot leak into a later id. */ +export function completeWorkspaceTransfer(transferId: string | undefined): void { + if (transferId) { + cancelledWorkspaceTransfers.delete(transferId); + } +} + +function isWorkspaceTransferCancelled(transferId: string | undefined): boolean { + return Boolean(transferId && cancelledWorkspaceTransfers.has(transferId)); +} + +function throwIfCancelled(transferId: string | undefined): void { + if (isWorkspaceTransferCancelled(transferId)) { + throw new Error(i18nService.t("panels/files:transfer.cancelled")); + } +} + export async function writeAllToLocalFile( destinationPath: string, chunks: AsyncIterable, @@ -134,7 +182,9 @@ export async function* readPeerFileChunks( sourcePath: string, onFileSize: (size: number) => void, identity: PeerFileWorkspaceIdentity, + transferId?: string, ): AsyncGenerator { + throwIfCancelled(transferId); const info = await adapter.requestPeerCommand({ cmd: "get_file_info", path: sourcePath, @@ -149,6 +199,8 @@ export async function* readPeerFileChunks( let offset = 0; let revision: string | undefined; while (offset < info.size) { + // A stop request must end the chunk loop instead of reading the whole file. + throwIfCancelled(transferId); const response = await adapter.requestPeerCommand({ cmd: "read_file_chunk", path: sourcePath, @@ -183,6 +235,9 @@ export async function* readPeerFileChunks( offset += bytes.byteLength; yield bytes; } + // The sink resumes this generator before publishing the staged file. A stop + // during the last chunk (or an empty file's info request) must still abort. + throwIfCancelled(transferId); return info.size; } @@ -190,15 +245,18 @@ async function collectPeerDirectoryEntries( sourceDirectory: string, destinationDirectory: string, remoteConnectionId?: string, + transferId?: string, ): Promise { const { mkdir } = await import("@tauri-apps/plugin-fs"); const pending = [{ source: sourceDirectory, destination: destinationDirectory }]; const files: PeerDownloadEntry[] = []; while (pending.length > 0) { + throwIfCancelled(transferId); const current = pending.shift()!; await mkdir(current.destination, { recursive: true }); const children = await workspaceAPI.getDirectoryChildren(current.source, remoteConnectionId ?? ''); + throwIfCancelled(transferId); for (const child of children) { if (!isSafePeerTransferEntryName(child.name)) { throw new Error(`Unsafe peer file name: '${child.name}'`); @@ -231,6 +289,7 @@ async function downloadPeerWorkspacePathToDisk( workspace: WorkspaceInfo | null, isDirectory: boolean, onProgress: (state: TransferProgressState | null) => void, + transferId?: string, ): Promise { if (!workspace?.id) throw new Error("A fixed peer workspace is required for download"); if (isRemoteWorkspace(workspace) && !workspace.connectionId) { @@ -244,7 +303,7 @@ async function downloadPeerWorkspacePathToDisk( remote_connection_id: isRemoteWorkspace(workspace) ? workspace.connectionId : undefined, }; const entries = isDirectory - ? await collectPeerDirectoryEntries(sourcePath, destinationPath, identity.remote_connection_id) + ? await collectPeerDirectoryEntries(sourcePath, destinationPath, identity.remote_connection_id, transferId) : [{ sourcePath, destinationPath, @@ -258,6 +317,7 @@ async function downloadPeerWorkspacePathToDisk( let smoothedSpeed = 0; for (const entry of entries) { + throwIfCancelled(transferId); const entryStart = bytesTransferred; let entryWritten = 0; let expectedEntrySize = entry.size; @@ -265,7 +325,13 @@ async function downloadPeerWorkspacePathToDisk( bytesTotal += size - expectedEntrySize; expectedEntrySize = size; }; - const chunks = readPeerFileChunks(adapter, entry.sourcePath, updateExpectedEntrySize, identity); + const chunks = readPeerFileChunks( + adapter, + entry.sourcePath, + updateExpectedEntrySize, + identity, + transferId, + ); await writeAllToLocalFile(entry.destinationPath, chunks, (written) => { entryWritten += written; bytesTransferred = entryStart + entryWritten; @@ -292,11 +358,13 @@ async function downloadPeerWorkspacePathToDisk( bytesTransferred, bytesTotal: Math.max(bytesTotal, bytesTransferred), speed: smoothedSpeed, + ...(transferId ? { transferId } : {}), }); }); bytesTransferred = entryStart + entryWritten; } + throwIfCancelled(transferId); onProgress({ phase: "download", current: Math.max(bytesTransferred, 1), @@ -306,6 +374,7 @@ async function downloadPeerWorkspacePathToDisk( bytesTransferred, bytesTotal: bytesTransferred, speed: smoothedSpeed, + ...(transferId ? { transferId } : {}), }); } @@ -584,6 +653,7 @@ export async function downloadWorkspaceFileToDisk( indeterminate: true, }); try { + throwIfCancelled(transferId); const peerAdapter = currentPeerAdapter(); if (peerAdapter) { await downloadPeerWorkspacePathToDisk( @@ -593,6 +663,7 @@ export async function downloadWorkspaceFileToDisk( workspace, Boolean(isDirectory), onProgress, + transferId, ); } else if (isRemoteWorkspace(workspace)) { const cid = workspace?.connectionId; @@ -639,7 +710,7 @@ export async function downloadWorkspaceFileToDisk( speed: smoothedSpeed, }); } - }, transferId); + }, transferId, () => isWorkspaceTransferCancelled(transferId)); } else { await workspaceAPI.exportLocalFileToPath(filePath, dest, workspace?.id); } @@ -649,8 +720,12 @@ export async function downloadWorkspaceFileToDisk( total: 1, label: baseName, indeterminate: false, + ...(transferId ? { transferId } : {}), }); } finally { + // The id is only meaningful while this download runs; a later transfer + // must not inherit its stop mark. + completeWorkspaceTransfer(transferId); window.setTimeout(() => onProgress(null), 450); } } @@ -691,6 +766,11 @@ export async function uploadLocalPathsToWorkspaceDirectory( const total = normalizedLocalPaths.length; for (let i = 0; i < total; i++) { + // A stop ends the whole drop session: the remaining items must not start + // a fresh backend transfer that no card is watching any more. + if (isWorkspaceTransferCancelled(transferId)) { + break; + } const localPath = normalizedLocalPaths[i]!; const name = localPath.split(/[/\\]/).pop(); if (!name) { @@ -713,7 +793,11 @@ export async function uploadLocalPathsToWorkspaceDirectory( total, label: singleItem ? name : `${name} (${i + 1}/${total})`, indeterminate: singleItem, + ...(transferId ? { transferId } : {}), }); + if (isWorkspaceTransferCancelled(transferId)) { + break; + } try { if (singleItem) { @@ -755,23 +839,37 @@ export async function uploadLocalPathsToWorkspaceDirectory( bytesTransferred: uploaded, bytesTotal: totalBytes, speed: smoothedSpeed, + ...(transferId ? { transferId } : {}), }); } }, transferId, + () => isWorkspaceTransferCancelled(transferId), ); successCount += 1; if (uploadResult.wasDirectory) { directoryCount += 1; } } else { - const uploadResult = await sshApi.uploadFromLocalPath(cid, localPath, destPath); + // Multi-item uploads share one transfer id, so the visible stop + // action reaches whichever item is currently being sent. + const uploadResult = await sshApi.uploadFromLocalPath( + cid, + localPath, + destPath, + undefined, + transferId, + () => isWorkspaceTransferCancelled(transferId), + ); successCount += 1; if (uploadResult.wasDirectory) { directoryCount += 1; } } } catch (error) { + if (isWorkspaceTransferCancelled(transferId)) { + break; + } failedFiles.push({ path: localPath, error: error instanceof Error ? error.message : String(error), @@ -779,13 +877,18 @@ export async function uploadLocalPathsToWorkspaceDirectory( } } - onProgress({ - phase: "upload", - current: total, - total, - label: "", - indeterminate: false, - }); + const wasCancelled = isWorkspaceTransferCancelled(transferId); + if (!wasCancelled) { + onProgress({ + phase: "upload", + current: total, + total, + label: "", + indeterminate: false, + ...(transferId ? { transferId } : {}), + }); + } + completeWorkspaceTransfer(transferId); window.setTimeout(() => onProgress(null), 450); if (successCount === 0 && failedFiles.length > 0) { @@ -795,9 +898,12 @@ export async function uploadLocalPathsToWorkspaceDirectory( throw new Error(details); } - return { successCount, directoryCount, failedFiles }; + return { successCount, directoryCount, failedFiles, cancelled: wasCancelled }; } + // A local paste is not cancellable: the clipboard helper owns it, and no + // stop mark may linger for an id this module never registered. + completeWorkspaceTransfer(transferId); onProgress({ phase: "upload", current: 0,