Refuse trailing-separator destination and normalize path spelling

CHANGELOG.md

@@ -0,0 +1,17 @@
+# Changelog
+
+All notable changes to Orbit are recorded here. The format follows
+[Keep a Changelog](https://keepachangelog.com/en/1.1.0/) and this project
+adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
+
+## [Unreleased]
+
+### Fixed
+
+- `orbit copy` and `orbit plan` now refuse a destination that ends with a
+  path separator (exit code 3, `destination_has_trailing_separator`). A
+  trailing separator is a directory signal, so silently normalizing it to a
+  single-file destination was surprising. Fixes #110.
+- Windows error messages render paths with a consistent ordinary spelling.
+  Reversible `\\?\C:\` and `\\?\UNC\` prefixes are now stripped before
+  display so similar errors read the same way. Fixes #111.

src/error.rs

@@ -15,6 +15,8 @@ pub enum OrbitError {
     DestinationExists(PathBuf),
     #[error("destination parent does not exist or is not a directory: {0}")]
     DestinationParentMissing(PathBuf),
+    #[error("destination has a trailing path separator, expected a single file path: {0}")]
+    DestinationHasTrailingSeparator(PathBuf),
     #[error("staged copy did not match the source: {0}")]
     VerificationFailed(PathBuf),
     #[error("source changed while it was being copied: {0}")]
@@ -45,6 +47,7 @@ impl OrbitError {
             Self::CopyRequiresRegularFile(_) => "copy_requires_regular_file",
             Self::DestinationExists(_) => "destination_exists",
             Self::DestinationParentMissing(_) => "destination_parent_missing",
+            Self::DestinationHasTrailingSeparator(_) => "destination_has_trailing_separator",
             Self::VerificationFailed(_) => "verification_failed",
             Self::SourceChanged(_) => "source_changed",
             Self::StagingChanged(_) => "staging_changed",
@@ -61,6 +64,7 @@ impl OrbitError {
             | Self::CopyRequiresRegularFile(_)
             | Self::DestinationExists(_)
             | Self::DestinationParentMissing(_)
+            | Self::DestinationHasTrailingSeparator(_)
             | Self::SourceChanged(_) => 3,
             Self::VerificationFailed(_) | Self::StagingChanged(_) | Self::StagingCleanup { .. } => {
                 1

src/paths.rs

@@ -20,6 +20,11 @@ pub fn resolve_endpoints(
     destination: &Path,
     cwd: &Path,
 ) -> Result<ResolvedEndpoints> {
+    if ends_with_separator(destination) {
+        return Err(OrbitError::DestinationHasTrailingSeparator(
+            destination.to_path_buf(),
+        ));
+    }
     let source_input = absolutize(source, cwd)?;
     let source_metadata = match fs::symlink_metadata(&source_input) {
         Ok(metadata) => metadata,
@@ -49,6 +54,22 @@ pub fn resolve_endpoints(
     })
 }
 
+#[cfg(windows)]
+fn ends_with_separator(path: &Path) -> bool {
+    // The raw user input must be inspected here because GetFullPathNameW drops
+    // trailing separators before resolve_leaf can see them.
+    path.as_os_str()
+        .encode_wide()
+        .last()
+        .is_some_and(|unit| unit == u16::from(b'\\') || unit == u16::from(b'/'))
+}
+
+#[cfg(not(windows))]
+fn ends_with_separator(path: &Path) -> bool {
+    use std::os::unix::ffi::OsStrExt;
+    matches!(path.as_os_str().as_bytes().last(), Some(&b'/'))
+}
+
 fn absolutize(path: &Path, cwd: &Path) -> Result<PathBuf> {
     let path = if path.is_absolute() {
         path.to_path_buf()
@@ -287,6 +308,33 @@ mod tests {
         ));
     }
 
+    #[test]
+    fn rejects_destination_with_trailing_separator() {
+        let temp = tempdir().unwrap();
+        let source = temp.path().join("source.txt");
+        fs::write(&source, b"source").unwrap();
+        // `\\` is a path separator on Windows but a literal filename character
+        // on Unix, so only check the separators the host actually uses.
+        let suffixes: &[&str] = if cfg!(windows) {
+            &["new_dir/", "new_dir\\"]
+        } else {
+            &["new_dir/"]
+        };
+        for suffix in suffixes {
+            let mut raw = temp.path().as_os_str().to_os_string();
+            raw.push(std::path::MAIN_SEPARATOR_STR);
+            raw.push(suffix);
+            let destination = std::path::PathBuf::from(raw);
+            assert!(
+                matches!(
+                    resolve_endpoints(&source, &destination, temp.path()),
+                    Err(OrbitError::DestinationHasTrailingSeparator(_))
+                ),
+                "destination {destination:?} slipped through trailing-separator check"
+            );
+        }
+    }
+
     #[test]
     fn rejects_destination_inside_source_directory() {
         let temp = tempdir().unwrap();

src/report.rs

@@ -186,6 +186,10 @@ fn error_message(error: &OrbitError) -> String {
             "destination parent does not exist or is not a directory: {}",
             display_path(path)
         ),
+        OrbitError::DestinationHasTrailingSeparator(path) => format!(
+            "destination has a trailing path separator, expected a single file path: {}",
+            display_path(path)
+        ),
         OrbitError::VerificationFailed(path) => format!(
             "staged copy did not match the source: {}",
             display_path(path)
@@ -304,13 +308,41 @@ fn write_json_line(writer: &mut impl Write, value: &impl serde::Serialize) -> io
     writeln!(writer)
 }
 
+#[cfg(not(windows))]
 fn display_path(path: &Path) -> String {
     match path.to_str() {
         Some(value) => value.to_owned(),
         None => format!("[{}:{}]", native_path_encoding(), native_path_hex(path)),
     }
 }
 
+#[cfg(windows)]
+fn display_path(path: &Path) -> String {
+    let Some(value) = path.to_str() else {
+        return format!("[{}:{}]", native_path_encoding(), native_path_hex(path));
+    };
+    let Some(rest) = value.strip_prefix(r"\\?\") else {
+        return value.to_owned();
+    };
+    if let Some(unc) = rest.strip_prefix("UNC\\") {
+        return format!(r"\\{unc}");
+    }
+    let bytes = rest.as_bytes();
+    let drive_letter = bytes
+        .first()
+        .copied()
+        .is_some_and(|c| c.is_ascii_alphabetic());
+    let drive_colon = bytes.get(1).copied() == Some(b':');
+    let drive_root = bytes
+        .get(2)
+        .copied()
+        .is_none_or(|c| c == b'\\' || c == b'/');
+    if drive_letter && drive_colon && drive_root {
+        return rest.to_owned();
+    }
+    value.to_owned()
+}
+
 #[cfg(unix)]
 fn native_path_encoding() -> &'static str {
     "unix_bytes_hex"
@@ -510,6 +542,45 @@ mod tests {
         );
     }
 
+    #[cfg(windows)]
+    #[test]
+    fn display_path_strips_reversible_verbatim_prefix() {
+        use super::display_path;
+        use std::path::PathBuf;
+
+        assert_eq!(
+            display_path(&PathBuf::from(r"\\?\C:\tmp\vibe\noexist")),
+            r"C:\tmp\vibe\noexist"
+        );
+        assert_eq!(display_path(&PathBuf::from(r"\\?\C:\")), r"C:\");
+        assert_eq!(
+            display_path(&PathBuf::from(r"\\?\UNC\server\share\file.txt")),
+            r"\\server\share\file.txt"
+        );
+        // Non-reversible verbatim forms stay as the operating system reported them.
+        let volume = r"\\?\Volume{00000000-0000-0000-0000-000000000000}\file.txt";
+        assert_eq!(display_path(&PathBuf::from(volume)), volume);
+        assert_eq!(
+            display_path(&PathBuf::from(r"C:\tmp\vibe\missing.txt")),
+            r"C:\tmp\vibe\missing.txt"
+        );
+    }
+
+    #[test]
+    fn trailing_separator_error_message_names_the_destination() {
+        use super::error_message;
+        use std::path::PathBuf;
+
+        let message = error_message(&OrbitError::DestinationHasTrailingSeparator(PathBuf::from(
+            "out/",
+        )));
+        assert!(
+            message.contains("trailing path separator"),
+            "message was: {message}"
+        );
+        assert!(message.contains("out/"), "message was: {message}");
+    }
+
     #[test]
     fn every_error_variant_has_a_stable_code_and_exit_code() {
         let errors = [

tests/copy_integration.rs

@@ -128,6 +128,28 @@ fn missing_destination_parent_is_refused_without_creating_directories() {
     assert!(partials(temp.path()).is_empty());
 }
 
+#[test]
+fn trailing_separator_destination_is_refused_without_creating_a_file() {
+    let temp = tempfile::tempdir().unwrap();
+    fs::write(temp.path().join("source.bin"), b"payload").unwrap();
+
+    #[cfg(windows)]
+    let destinations = ["new_dir/", "new_dir\\"];
+    #[cfg(not(windows))]
+    let destinations = ["new_dir/"];
+
+    for destination in destinations {
+        orbit(temp.path())
+            .args(["copy", "source.bin", destination])
+            .assert()
+            .code(3)
+            .stderr(predicate::str::contains("trailing path separator"));
+
+        assert!(!temp.path().join("new_dir").exists());
+        assert!(partials(temp.path()).is_empty());
+    }
+}
+
 #[test]
 fn json_success_is_one_machine_readable_result() {
     let temp = tempfile::tempdir().unwrap();
@@ -185,6 +207,29 @@ fn quiet_refusal_remains_visible_on_stderr() {
         .stderr(predicate::str::contains("destination already exists"));
 }
 
+#[cfg(windows)]
+#[test]
+fn error_messages_use_ordinary_path_spelling_on_windows() {
+    let temp = tempfile::tempdir().unwrap();
+    fs::write(temp.path().join("source.bin"), b"payload").unwrap();
+
+    let output = orbit(temp.path())
+        .args(["copy", "source.bin", "missing/destination.bin"])
+        .output()
+        .unwrap();
+
+    assert_eq!(output.status.code(), Some(3));
+    let stderr = String::from_utf8(output.stderr).unwrap();
+    assert!(
+        !stderr.contains(r"\\?\"),
+        "error message leaked verbatim prefix: {stderr}"
+    );
+    assert!(
+        stderr.contains("destination parent"),
+        "unexpected message: {stderr}"
+    );
+}
+
 #[test]
 fn symbolic_link_source_is_refused_without_following_it() {
     let temp = tempfile::tempdir().unwrap();

tests/plan_integration.rs

@@ -426,13 +426,41 @@ fn quiet_conflict_reports_exact_blocking_paths_and_reasons_on_stderr() {
         String::from_utf8(output.stderr).unwrap(),
         format!(
             "error: CONFLICT {} — destination file has different contents\nerror: CONFLICT {} — destination file has different contents\n",
-            resolved_destination.join("a.txt").to_str().unwrap(),
-            resolved_destination.join("b.txt").to_str().unwrap(),
+            display_path(&resolved_destination.join("a.txt")),
+            display_path(&resolved_destination.join("b.txt")),
         )
     );
     assert_tree_unchanged(temp.path(), &before);
 }
 
+/// Mirrors `report::display_path` on Windows: reversible `\\?\` prefixes are
+/// stripped so human error output matches an ordinary path spelling.
+fn display_path(path: &Path) -> String {
+    let value = path.to_str().expect("test paths are UTF-8");
+    #[cfg(windows)]
+    {
+        if let Some(rest) = value.strip_prefix(r"\\?\") {
+            if let Some(unc) = rest.strip_prefix("UNC\\") {
+                return format!(r"\\{unc}");
+            }
+            let bytes = rest.as_bytes();
+            let drive_letter = bytes
+                .first()
+                .copied()
+                .is_some_and(|c| c.is_ascii_alphabetic());
+            let drive_colon = bytes.get(1).copied() == Some(b':');
+            let drive_root = bytes
+                .get(2)
+                .copied()
+                .is_none_or(|c| c == b'\\' || c == b'/');
+            if drive_letter && drive_colon && drive_root {
+                return rest.to_owned();
+            }
+        }
+    }
+    value.to_owned()
+}
+
 #[cfg(unix)]
 #[test]
 fn quiet_unsupported_socket_reports_source_and_can_be_explicitly_ignored() {