From 20847a0de8e30f931a9d24c802577d6bbef2640d Mon Sep 17 00:00:00 2001 From: Harold Hunt Date: Fri, 23 Oct 2026 21:00:04 -0400 Subject: [PATCH 1/4] ci: fix the macOS and Windows failures on main Windows clients rejected every path an SSH host sent. The helper is Unix-only, so remote roots are /-rooted, but Path::is_absolute also wants a drive letter on Windows. Root discovery, the scan header and the picker now check remote paths with is_remote_absolute, which agrees with is_absolute on Unix. The volume picker test compared against the unresolved temporary path. Accepting a local root canonicalizes it, and the temporary directory is /var -> /private/var on macOS and an 8.3 short name on Windows runners. spawn_command and wait in remote.rs are only used by the Unix tests, so clippy -D warnings failed on Windows. Gate them on unix. --- crates/disktree-app/src/state.rs | 7 +++++- crates/disktree-app/src/tests.rs | 5 +++- crates/disktree-core/src/remote.rs | 37 ++++++++++++++++++++++++++---- 3 files changed, 42 insertions(+), 7 deletions(-) diff --git a/crates/disktree-app/src/state.rs b/crates/disktree-app/src/state.rs index efb9730a..4fbe020f 100644 --- a/crates/disktree-app/src/state.rs +++ b/crates/disktree-app/src/state.rs @@ -948,7 +948,12 @@ impl Disktree { cx.notify(); return; } - if !path.is_absolute() { + let absolute = if self.volumes_source.is_some() { + disktree_core::remote::is_remote_absolute(&path) + } else { + path.is_absolute() + }; + if !absolute { self.picker_error = Some("Enter an absolute directory path.".into()); cx.notify(); diff --git a/crates/disktree-app/src/tests.rs b/crates/disktree-app/src/tests.rs index 0520e4cb..758db92a 100644 --- a/crates/disktree-app/src/tests.rs +++ b/crates/disktree-app/src/tests.rs @@ -1014,7 +1014,10 @@ fn enter_in_the_focused_volume_picker_scans_the_selected_root( ) { cx.update(gpui_omarchy::init); let temp = fixture(); - let target = temp.path().join("junk"); + // Accepting a local root canonicalizes it, and the temporary directory + // is behind a link on macOS (/var -> /private/var) and a short 8.3 name + // on Windows runners, so compare against the resolved path. + let target = dunce::canonicalize(temp.path().join("junk")).unwrap(); let (view, cx) = view_over(temp.path(), cx); update(&view, cx, |app, _| { app.volumes = vec![disktree_core::space::Volume { diff --git a/crates/disktree-core/src/remote.rs b/crates/disktree-core/src/remote.rs index 27da007f..7e22c791 100644 --- a/crates/disktree-core/src/remote.rs +++ b/crates/disktree-core/src/remote.rs @@ -5,7 +5,7 @@ //! and stream without making a second serialized copy of the whole tree. use std::io::{self, BufRead, BufReader, Read as _, Write}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::process::{Child, Command, Stdio}; use std::sync::{Arc, Mutex, mpsc}; @@ -20,6 +20,18 @@ use crate::tree::{Node, NodeKind, aggregate}; use std::time::Instant; const VERSION: u32 = 1; + +/// Whether `path` names a directory from the remote host's root. +/// +/// The SSH helper is Unix-only, so remote paths are always `/`-rooted. +/// `Path::is_absolute` also wants a drive letter on Windows, which would +/// reject every remote path a Windows client receives. On Unix the two +/// checks agree. +#[must_use] +pub fn is_remote_absolute(path: &Path) -> bool { + path.has_root() +} + const MAX_LINE: u64 = 1 << 20; const MAX_DIAGNOSTICS: usize = 16 << 10; @@ -84,8 +96,11 @@ pub fn read_roots(input: &mut impl BufRead) -> io::Result { || roots .volumes .iter() - .any(|volume| !volume.point.is_absolute()) - || roots.home.as_ref().is_some_and(|home| !home.is_absolute()) + .any(|volume| !is_remote_absolute(&volume.point)) + || roots + .home + .as_ref() + .is_some_and(|home| !is_remote_absolute(home)) { return Err(io::Error::other("incompatible remote root discovery")); } @@ -312,7 +327,7 @@ fn read_snapshot_with_progress( shared: &Shared, ) -> io::Result { let header: Header = receive(input)?; - if header.version != VERSION || !header.root.is_absolute() { + if header.version != VERSION || !is_remote_absolute(&header.root) { return Err(io::Error::other("incompatible remote helper protocol")); } let mut progress = Progress { @@ -577,7 +592,8 @@ impl RemoteScan { Self::spawn_with_archive(command, host, request, archive) } - #[cfg(test)] + // Only the Unix tests run a fake ssh, so Windows has no caller. + #[cfg(all(test, unix))] fn spawn_command( command: Command, host: &str, @@ -744,6 +760,7 @@ mod tests { } } + #[cfg(unix)] fn wait(scan: &RemoteJob) -> io::Result { wait_for(scan, std::time::Duration::from_secs(10)) } @@ -1086,6 +1103,16 @@ mod tests { ); } + /// Remote paths come from a Unix host, so a Windows client must accept + /// them without a drive letter. + #[test] + fn remote_paths_are_absolute_without_a_drive_letter() { + assert!(is_remote_absolute(Path::new("/"))); + assert!(is_remote_absolute(Path::new("/home/someone"))); + assert!(!is_remote_absolute(Path::new("home/someone"))); + assert!(!is_remote_absolute(Path::new(""))); + } + #[test] fn rejects_ssh_options_and_shell_syntax() { for host in ["-oProxyCommand=oops", "dev; touch x", "dev\nother", ""] { From 43d94b07adeb15adeb1db96fcf859bf2a07d7981 Mon Sep 17 00:00:00 2001 From: Harold Hunt Date: Fri, 23 Oct 2026 21:04:22 -0400 Subject: [PATCH 2/4] ssh: accept only /-rooted remote paths on every client Path::has_root also accepts \x and C:\x on Windows, which name nothing on the Unix host the helper runs on. Check for a leading / instead. --- crates/disktree-core/src/remote.rs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/crates/disktree-core/src/remote.rs b/crates/disktree-core/src/remote.rs index 7e22c791..0521ff13 100644 --- a/crates/disktree-core/src/remote.rs +++ b/crates/disktree-core/src/remote.rs @@ -20,21 +20,20 @@ use crate::tree::{Node, NodeKind, aggregate}; use std::time::Instant; const VERSION: u32 = 1; +const MAX_LINE: u64 = 1 << 20; +const MAX_DIAGNOSTICS: usize = 16 << 10; /// Whether `path` names a directory from the remote host's root. /// -/// The SSH helper is Unix-only, so remote paths are always `/`-rooted. -/// `Path::is_absolute` also wants a drive letter on Windows, which would -/// reject every remote path a Windows client receives. On Unix the two -/// checks agree. +/// The SSH helper is Unix-only, so remote paths are `/`-rooted. +/// `Path::is_absolute` also wants a drive letter on Windows, and +/// `Path::has_root` accepts `\x` and `C:\x` there, so neither works for a +/// Windows client. On Unix this agrees with `is_absolute`. #[must_use] pub fn is_remote_absolute(path: &Path) -> bool { - path.has_root() + path.as_os_str().as_encoded_bytes().first() == Some(&b'/') } -const MAX_LINE: u64 = 1 << 20; -const MAX_DIAGNOSTICS: usize = 16 << 10; - // Non-interactive shells often omit ~/.local/bin. Do not require users to // edit the server's startup files after the documented user-local install. const REMOTE_COMMAND: &str = r#"if command -v disktree-scan >/dev/null 2>&1; then @@ -1111,6 +1110,10 @@ mod tests { assert!(is_remote_absolute(Path::new("/home/someone"))); assert!(!is_remote_absolute(Path::new("home/someone"))); assert!(!is_remote_absolute(Path::new(""))); + // Windows-rooted paths name nothing on a Unix host. + assert!(!is_remote_absolute(Path::new(r"\home\someone"))); + assert!(!is_remote_absolute(Path::new(r"C:\Users"))); + assert!(!is_remote_absolute(Path::new(r"\\server\share"))); } #[test] From 3b972817c85d46a88582ca6310b07c56567b6c58 Mon Sep 17 00:00:00 2001 From: Harold Hunt Date: Fri, 23 Oct 2026 21:17:30 -0400 Subject: [PATCH 3/4] ssh: accept a Windows host's absolute paths again serve also runs on Windows, and the round-trip tests there send native C:\ roots. Accept a path that is absolute on this platform or /-rooted; \x stays rejected. --- crates/disktree-core/src/remote.rs | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/crates/disktree-core/src/remote.rs b/crates/disktree-core/src/remote.rs index 0521ff13..6877d2ef 100644 --- a/crates/disktree-core/src/remote.rs +++ b/crates/disktree-core/src/remote.rs @@ -25,13 +25,15 @@ const MAX_DIAGNOSTICS: usize = 16 << 10; /// Whether `path` names a directory from the remote host's root. /// -/// The SSH helper is Unix-only, so remote paths are `/`-rooted. -/// `Path::is_absolute` also wants a drive letter on Windows, and -/// `Path::has_root` accepts `\x` and `C:\x` there, so neither works for a -/// Windows client. On Unix this agrees with `is_absolute`. +/// `serve` runs on Windows as well as Unix, so a remote path is either +/// `/`-rooted or absolute the Windows way. On Windows +/// `Path::is_absolute` wants a drive letter, which rejects every Unix path, +/// and `Path::has_root` also accepts `\x`, which is neither. On Unix this +/// agrees with `is_absolute`. #[must_use] pub fn is_remote_absolute(path: &Path) -> bool { - path.as_os_str().as_encoded_bytes().first() == Some(&b'/') + path.is_absolute() + || path.as_os_str().as_encoded_bytes().first() == Some(&b'/') } // Non-interactive shells often omit ~/.local/bin. Do not require users to @@ -1102,18 +1104,17 @@ mod tests { ); } - /// Remote paths come from a Unix host, so a Windows client must accept - /// them without a drive letter. + /// A Windows client must accept a Unix host's paths without a drive + /// letter, and every client accepts its own platform's absolute paths. #[test] fn remote_paths_are_absolute_without_a_drive_letter() { assert!(is_remote_absolute(Path::new("/"))); assert!(is_remote_absolute(Path::new("/home/someone"))); assert!(!is_remote_absolute(Path::new("home/someone"))); assert!(!is_remote_absolute(Path::new(""))); - // Windows-rooted paths name nothing on a Unix host. + // Rooted without a drive is absolute nowhere. assert!(!is_remote_absolute(Path::new(r"\home\someone"))); - assert!(!is_remote_absolute(Path::new(r"C:\Users"))); - assert!(!is_remote_absolute(Path::new(r"\\server\share"))); + assert_eq!(is_remote_absolute(Path::new(r"C:\Users")), cfg!(windows)); } #[test] From cc026ea76197c7d3fba6a4ed61b5726ae8ddf9d8 Mon Sep 17 00:00:00 2001 From: Harold Hunt Date: Fri, 23 Oct 2026 22:03:33 -0400 Subject: [PATCH 4/4] test: run the live preview dedup test on Unix only The preview comes from the Unix helper's streaming walk. On Windows serve sends a finished tree with no preview, so the test's unwrap failed there. It was hidden on main because cargo test stops at the first failing test binary, which was the app's. --- crates/disktree-core/src/remote.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/disktree-core/src/remote.rs b/crates/disktree-core/src/remote.rs index 6877d2ef..8d2881a4 100644 --- a/crates/disktree-core/src/remote.rs +++ b/crates/disktree-core/src/remote.rs @@ -1205,6 +1205,9 @@ mod tests { assert_eq!(shared.lock().unwrap().stage, Stage::Finalizing); } + // The live preview this checks comes from the Unix helper's streaming + // walk; on other systems `serve` sends a finished tree with no preview. + #[cfg(unix)] #[test] fn desktop_dedup_counts_links_once_without_requiring_external_names() { let dir = tempfile::tempdir().unwrap();