fix(cli): preserve directory basename for filtered uploads (#1028)

Signed-off-by: John Myers <johntmyers@users.noreply.github.com>
Co-authored-by: John Myers <johntmyers@users.noreply.github.com>

Author: johntmyers

architecture/sandbox-connect.md

@@ -356,6 +356,8 @@ openshell sandbox download <name> <sandbox-path> [<local-path>]
 ```
 
 - **Upload**: `sandbox_upload()` streams a tar archive of the local path to `ssh ... tar xf - -C <dest>` on the sandbox side. Default destination: `/sandbox`.
+  Named directory uploads preserve the source directory basename at the destination, matching `scp -r` and `cp -r`; uploading `.` remains flat.
+  `.gitignore` filtering only changes which files are included, not the destination layout.
 - **Download**: `sandbox_download()` runs `ssh ... tar cf - -C <dir> <path>` on the sandbox side and extracts the output locally via `tar::Archive`. Default destination: `.` (current directory).
 - No compression for v1 -- the SSH tunnel rides the already-TLS-encrypted gateway connection; compression adds CPU cost with marginal bandwidth savings.
 

crates/openshell-cli/src/main.rs

@@ -2465,6 +2465,7 @@ async fn main() -> Result<()> {
                             &name,
                             &base_dir,
                             &files,
+                            local,
                             sandbox_dest,
                             &tls,
                         )

crates/openshell-cli/src/run.rs

@@ -2417,6 +2417,7 @@ pub async fn sandbox_create(
                         &sandbox_name,
                         &base_dir,
                         &files,
+                        local,
                         dest,
                         &effective_tls,
                     )

crates/openshell-cli/src/ssh.rs

@@ -16,7 +16,7 @@ use openshell_core::proto::{CreateSshSessionRequest, GetSandboxRequest};
 use owo_colors::OwoColorize;
 use rustls::pki_types::ServerName;
 use std::fs;
-use std::io::IsTerminal;
+use std::io::{IsTerminal, Write};
 #[cfg(unix)]
 use std::os::unix::process::CommandExt;
 use std::path::{Path, PathBuf};
@@ -476,9 +476,68 @@ enum UploadSource {
     FileList {
         base_dir: PathBuf,
         files: Vec<String>,
+        archive_prefix: Option<PathBuf>,
     },
 }
 
+fn write_upload_archive<W: Write>(writer: W, source: UploadSource) -> Result<()> {
+    let mut archive = tar::Builder::new(writer);
+    match source {
+        UploadSource::SinglePath {
+            local_path,
+            tar_name,
+        } => {
+            if local_path.is_file() {
+                archive
+                    .append_path_with_name(&local_path, &tar_name)
+                    .into_diagnostic()?;
+            } else if local_path.is_dir() {
+                archive
+                    .append_dir_all(&tar_name, &local_path)
+                    .into_diagnostic()?;
+            } else {
+                return Err(miette::miette!(
+                    "local path does not exist: {}",
+                    local_path.display()
+                ));
+            }
+        }
+        UploadSource::FileList {
+            base_dir,
+            files,
+            archive_prefix,
+        } => {
+            for file in &files {
+                let full_path = base_dir.join(file);
+                let archive_path = archive_prefix
+                    .as_ref()
+                    .map(|prefix| prefix.join(file))
+                    .unwrap_or_else(|| PathBuf::from(file));
+                if full_path.is_file() {
+                    archive
+                        .append_path_with_name(&full_path, &archive_path)
+                        .into_diagnostic()
+                        .wrap_err_with(|| {
+                            format!("failed to add {} to tar archive", archive_path.display())
+                        })?;
+                } else if full_path.is_dir() {
+                    archive
+                        .append_dir_all(&archive_path, &full_path)
+                        .into_diagnostic()
+                        .wrap_err_with(|| {
+                            format!(
+                                "failed to add directory {} to tar archive",
+                                archive_path.display()
+                            )
+                        })?;
+                }
+            }
+        }
+    }
+    archive.finish().into_diagnostic()?;
+    Ok(())
+}
+
 /// Core tar-over-SSH upload: streams a tar archive into `dest_dir` on the
 /// sandbox.  Callers are responsible for splitting the destination path so
 /// that `dest_dir` is always a directory.
@@ -521,52 +580,9 @@ async fn ssh_tar_upload(
         .ok_or_else(|| miette::miette!("failed to open stdin for ssh process"))?;
 
     // Build the tar archive in a blocking task since the tar crate is synchronous.
-    tokio::task::spawn_blocking(move || -> Result<()> {
-        let mut archive = tar::Builder::new(stdin);
-        match source {
-            UploadSource::SinglePath {
-                local_path,
-                tar_name,
-            } => {
-                if local_path.is_file() {
-                    archive
-                        .append_path_with_name(&local_path, &tar_name)
-                        .into_diagnostic()?;
-                } else if local_path.is_dir() {
-                    archive
-                        .append_dir_all(&tar_name, &local_path)
-                        .into_diagnostic()?;
-                } else {
-                    return Err(miette::miette!(
-                        "local path does not exist: {}",
-                        local_path.display()
-                    ));
-                }
-            }
-            UploadSource::FileList { base_dir, files } => {
-                for file in &files {
-                    let full_path = base_dir.join(file);
-                    if full_path.is_file() {
-                        archive
-                            .append_path_with_name(&full_path, file)
-                            .into_diagnostic()
-                            .wrap_err_with(|| format!("failed to add {file} to tar archive"))?;
-                    } else if full_path.is_dir() {
-                        archive
-                            .append_dir_all(file, &full_path)
-                            .into_diagnostic()
-                            .wrap_err_with(|| {
-                                format!("failed to add directory {file} to tar archive")
-                            })?;
-                    }
-                }
-            }
-        }
-        archive.finish().into_diagnostic()?;
-        Ok(())
-    })
-    .await
-    .into_diagnostic()??;
+    tokio::task::spawn_blocking(move || -> Result<()> { write_upload_archive(stdin, source) })
+        .await
+        .into_diagnostic()??;
 
     let status = tokio::task::spawn_blocking(move || child.wait())
         .await
@@ -606,6 +622,7 @@ pub async fn sandbox_sync_up_files(
     name: &str,
     base_dir: &Path,
     files: &[String],
+    local_path: &Path,
     dest: Option<&str>,
     tls: &TlsOptions,
 ) -> Result<()> {
@@ -619,6 +636,7 @@ pub async fn sandbox_sync_up_files(
         UploadSource::FileList {
             base_dir: base_dir.to_path_buf(),
             files: files.to_vec(),
+            archive_prefix: file_list_archive_prefix(local_path),
         },
         tls,
     )
@@ -706,6 +724,19 @@ fn directory_upload_prefix(local_path: &Path) -> std::ffi::OsString {
         .unwrap_or_else(|| ".".into())
 }
 
+fn file_list_archive_prefix(local_path: &Path) -> Option<PathBuf> {
+    if !local_path.is_dir() {
+        return None;
+    }
+
+    let prefix = directory_upload_prefix(local_path);
+    if prefix == "." {
+        None
+    } else {
+        Some(PathBuf::from(prefix))
+    }
+}
+
 /// Pull a path from a sandbox to a local destination using tar-over-SSH.
 pub async fn sandbox_sync_down(
     server: &str,
@@ -1332,6 +1363,93 @@ mod tests {
         );
     }
 
+    #[test]
+    fn file_list_archive_prefix_uses_named_directory_basename() {
+        let tmpdir = tempfile::tempdir().expect("create tmpdir");
+        let source = tmpdir.path().join("source-dir");
+        let file = tmpdir.path().join("file.txt");
+        fs::create_dir_all(&source).expect("create source dir");
+        fs::write(&file, "file").expect("write file");
+
+        assert_eq!(
+            file_list_archive_prefix(&source),
+            Some(PathBuf::from("source-dir"))
+        );
+        assert_eq!(file_list_archive_prefix(Path::new(".")), None);
+        assert_eq!(file_list_archive_prefix(&file), None);
+    }
+
+    fn upload_archive_paths(source: UploadSource) -> Vec<String> {
+        let mut bytes = Vec::new();
+        write_upload_archive(&mut bytes, source).expect("write upload archive");
+        let mut archive = tar::Archive::new(std::io::Cursor::new(bytes));
+        let entries = archive.entries().expect("read archive entries");
+        let mut paths = entries
+            .map(|entry| {
+                entry
+                    .expect("read archive entry")
+                    .path()
+                    .expect("read archive path")
+                    .to_string_lossy()
+                    .into_owned()
+            })
+            .collect::<Vec<_>>();
+        paths.sort();
+        paths
+    }
+
+    #[test]
+    fn file_list_archive_preserves_directory_prefix_when_requested() {
+        let tmpdir = tempfile::tempdir().expect("create tmpdir");
+        let base_dir = tmpdir.path().join("nested");
+        fs::create_dir_all(base_dir.join("inner")).expect("create dirs");
+        fs::write(base_dir.join("file.txt"), "file").expect("write file");
+        fs::write(base_dir.join("inner/child.txt"), "child").expect("write child");
+
+        let paths = upload_archive_paths(UploadSource::FileList {
+            base_dir,
+            files: vec!["file.txt".into(), "inner/child.txt".into()],
+            archive_prefix: Some(PathBuf::from("nested")),
+        });
+
+        assert_eq!(paths, vec!["nested/file.txt", "nested/inner/child.txt"]);
+    }
+
+    #[test]
+    fn file_list_archive_stays_flat_without_directory_prefix() {
+        let tmpdir = tempfile::tempdir().expect("create tmpdir");
+        let base_dir = tmpdir.path().join("nested");
+        fs::create_dir_all(base_dir.join("inner")).expect("create dirs");
+        fs::write(base_dir.join("file.txt"), "file").expect("write file");
+        fs::write(base_dir.join("inner/child.txt"), "child").expect("write child");
+
+        let paths = upload_archive_paths(UploadSource::FileList {
+            base_dir,
+            files: vec!["file.txt".into(), "inner/child.txt".into()],
+            archive_prefix: None,
+        });
+
+        assert_eq!(paths, vec!["file.txt", "inner/child.txt"]);
+    }
+
+    #[test]
+    fn single_directory_archive_preserves_directory_basename() {
+        let tmpdir = tempfile::tempdir().expect("create tmpdir");
+        let source = tmpdir.path().join("source-dir");
+        fs::create_dir_all(source.join("inner")).expect("create dirs");
+        fs::write(source.join("file.txt"), "file").expect("write file");
+        fs::write(source.join("inner/child.txt"), "child").expect("write child");
+
+        let paths = upload_archive_paths(UploadSource::SinglePath {
+            local_path: source,
+            tar_name: "source-dir".into(),
+        });
+
+        assert!(paths.contains(&"source-dir/file.txt".to_string()));
+        assert!(paths.contains(&"source-dir/inner/child.txt".to_string()));
+        assert!(paths.iter().all(|path| path.starts_with("source-dir/")));
+    }
+
     #[test]
     fn split_sandbox_path_handles_root_and_bare_names() {
         // File directly under root