From 01a8f5c503da77fa0e25206d6fd9f0dcf51bf937 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Tue, 30 Jun 2026 15:05:25 -0700 Subject: [PATCH] Apply remaining review findings (L-e, L-f) - L-e: route terminal file drops purely by a bounds hit-test instead of the `active` flag. Inactive panes are display:none (zero-size rect) so they never match; a zero-size guard makes that explicit. Correct for the current tabbed layout and future-proof for split panes, where a drop on a visible-but-unfocused pane previously matched no handler. - L-f: stream the dropped file straight into the upload tar inside a blocking task (new exec::upload_host_file_to_container) instead of reading the whole file into a Vec and then re-packing it. Peak memory drops from ~2x to ~1x the file size, and the synchronous file IO no longer runs on the async worker. cargo check / tsc / vitest all pass. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/commands/terminal_commands.rs | 13 +---- app/src-tauri/src/docker/exec.rs | 57 +++++++++++++++++++ app/src/components/terminal/TerminalView.tsx | 10 ++-- 3 files changed, 66 insertions(+), 14 deletions(-) diff --git a/app/src-tauri/src/commands/terminal_commands.rs b/app/src-tauri/src/commands/terminal_commands.rs index 82dc7fd..2df434f 100644 --- a/app/src-tauri/src/commands/terminal_commands.rs +++ b/app/src-tauri/src/commands/terminal_commands.rs @@ -203,8 +203,8 @@ pub async fn upload_host_file_to_terminal( return Err(format!("{} is a directory — drop individual files", host_path)); } - // Guard against ballooning host RAM: the file is read fully into memory and - // then re-packed into an in-memory tar, so cap the size of a dropped file. + // Guard against ballooning host RAM: the file is packed into an in-memory + // tar before upload, so cap the size of a dropped file. const MAX_DROP_BYTES: u64 = 256 * 1024 * 1024; // 256 MiB if meta.len() > MAX_DROP_BYTES { return Err(format!( @@ -214,10 +214,6 @@ pub async fn upload_host_file_to_terminal( )); } - let data = tokio::fs::read(&host_path) - .await - .map_err(|e| format!("Failed to read {}: {}", host_path, e))?; - let base = std::path::Path::new(&host_path) .file_name() .map(|s| s.to_string_lossy().to_string()) @@ -233,10 +229,7 @@ pub async fn upload_host_file_to_terminal( .await?; let file_name = format!("triple-c-drops/{}", base); - state - .exec_manager - .write_file_to_container(&container_id, &file_name, &data) - .await + crate::docker::exec::upload_host_file_to_container(&container_id, &host_path, &file_name).await } #[tauri::command] diff --git a/app/src-tauri/src/docker/exec.rs b/app/src-tauri/src/docker/exec.rs index 2cba63a..0d5b83a 100644 --- a/app/src-tauri/src/docker/exec.rs +++ b/app/src-tauri/src/docker/exec.rs @@ -279,6 +279,63 @@ impl ExecSessionManager { } } +/// Upload a host file into the container's `/tmp` under `dest_name`, building the +/// tar archive by streaming from the file inside a blocking task. Unlike reading +/// the whole file into a `Vec` and then handing it to `write_file_to_container` +/// (which holds two full-size copies — the data and the tar), this keeps only +/// the single tar buffer, and the synchronous file IO runs off the async worker. +/// Returns the in-container path (`/tmp/`). +pub async fn upload_host_file_to_container( + container_id: &str, + host_path: &str, + dest_name: &str, +) -> Result { + let host_path = host_path.to_string(); + let dest_name = dest_name.to_string(); + let dest_for_blk = dest_name.clone(); + + let tar_buf = tokio::task::spawn_blocking(move || -> Result, String> { + let mut file = std::fs::File::open(&host_path) + .map_err(|e| format!("Failed to read {}: {}", host_path, e))?; + let size = file + .metadata() + .map_err(|e| format!("Failed to stat {}: {}", host_path, e))? + .len(); + let mut tar_buf = Vec::new(); + { + let mut builder = tar::Builder::new(&mut tar_buf); + let mut header = tar::Header::new_gnu(); + header.set_size(size); + header.set_mode(0o644); + header.set_cksum(); + builder + .append_data(&mut header, &dest_for_blk, &mut file) + .map_err(|e| format!("Failed to create tar entry: {}", e))?; + builder + .finish() + .map_err(|e| format!("Failed to finalize tar: {}", e))?; + } + Ok(tar_buf) + }) + .await + .map_err(|e| format!("Upload task panicked: {}", e))??; + + let docker = get_docker()?; + docker + .upload_to_container( + container_id, + Some(UploadToContainerOptions { + path: "/tmp".to_string(), + ..Default::default() + }), + tar_buf.into(), + ) + .await + .map_err(|e| format!("Failed to upload file to container: {}", e))?; + + Ok(format!("/tmp/{}", dest_name)) +} + /// Run a one-shot (non-interactive) exec command in a container and collect stdout. pub async fn exec_oneshot(container_id: &str, cmd: Vec) -> Result { exec_oneshot_env(container_id, cmd, Vec::new()).await diff --git a/app/src/components/terminal/TerminalView.tsx b/app/src/components/terminal/TerminalView.tsx index d167211..5460015 100644 --- a/app/src/components/terminal/TerminalView.tsx +++ b/app/src/components/terminal/TerminalView.tsx @@ -57,15 +57,18 @@ export default function TerminalView({ sessionId, active }: Props) { // in-container paths typed into the prompt so Claude Code can read them. // Tauri intercepts OS file drops at the webview level, so we use // onDragDropEvent (HTML5 ondrop on the element wouldn't expose file paths). - // The listener is window-wide, so we guard on `active` + a hit-test against - // this terminal's bounds to ignore drops meant for another pane. + // The listener is window-wide, so we route purely by a hit-test against this + // terminal's bounds: the pane the drop lands on handles it. Inactive panes are + // `display:none` (zero-size rect) so they never match — this works for the + // current tabbed layout and would also do the right thing with split panes. useEffect(() => { let unlisten: (() => void) | undefined; let cancelled = false; const insideThisTerminal = (pos: { x: number; y: number }): boolean => { const rect = containerRef.current?.getBoundingClientRect(); - if (!rect) return false; + // A hidden (display:none) pane has a zero-size rect — never a drop target. + if (!rect || rect.width === 0 || rect.height === 0) return false; const dpr = window.devicePixelRatio || 1; const x = pos.x / dpr; const y = pos.y / dpr; @@ -80,7 +83,6 @@ export default function TerminalView({ sessionId, active }: Props) { (async () => { const un = await getCurrentWebview().onDragDropEvent(async (event) => { if (event.payload.type !== "drop") return; - if (!activeRef.current) return; if (!insideThisTerminal(event.payload.position)) return; const paths = event.payload.paths ?? [];