Apply remaining review findings (L-e, L-f)
Build App / compute-version (pull_request) Successful in 3s
Build Container / build-container (pull_request) Successful in 1m30s
Build App / build-macos (pull_request) Successful in 2m16s
Build App / build-windows (pull_request) Successful in 3m6s
Build App / build-linux (pull_request) Successful in 6m11s
Build App / create-tag (pull_request) Has been skipped
Build App / sync-to-github (pull_request) Has been skipped
Build App / compute-version (pull_request) Successful in 3s
Build Container / build-container (pull_request) Successful in 1m30s
Build App / build-macos (pull_request) Successful in 2m16s
Build App / build-windows (pull_request) Successful in 3m6s
Build App / build-linux (pull_request) Successful in 6m11s
Build App / create-tag (pull_request) Has been skipped
Build App / sync-to-github (pull_request) Has been skipped
- 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) <noreply@anthropic.com>
This commit is contained in:
@@ -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]
|
||||
|
||||
@@ -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/<dest_name>`).
|
||||
pub async fn upload_host_file_to_container(
|
||||
container_id: &str,
|
||||
host_path: &str,
|
||||
dest_name: &str,
|
||||
) -> Result<String, String> {
|
||||
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<Vec<u8>, 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<String>) -> Result<String, String> {
|
||||
exec_oneshot_env(container_id, cmd, Vec::new()).await
|
||||
|
||||
@@ -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 ?? [];
|
||||
|
||||
Reference in New Issue
Block a user