feat(hm-util): add atomic_rename_over with ReplaceFileW on Windows

Author: markovejnovic

crates/hm-util/src/os/fs.rs

@@ -9,10 +9,6 @@
 use std::io;
 use std::path::Path;
 
-// ---------------------------------------------------------------------------
-// Private sync core
-// ---------------------------------------------------------------------------
-
 fn write_atomic_restricted_sync(
     path: &Path,
     contents: &[u8],
@@ -101,9 +97,55 @@ fn write_file_with_mode_sync(path: &Path, contents: &[u8], _mode: u32) -> io::Re
 }
 
 // ---------------------------------------------------------------------------
-// Public async API
+// Cross-platform atomic rename
 // ---------------------------------------------------------------------------
 
+#[cfg(unix)]
+fn atomic_rename_over_sync(from: &Path, to: &Path) -> io::Result<()> {
+    std::fs::rename(from, to)
+}
+
+#[cfg(windows)]
+fn atomic_rename_over_sync(from: &Path, to: &Path) -> io::Result<()> {
+    use windows::core::HSTRING;
+    use windows::Win32::Storage::FileSystem::{
+        MoveFileExW, ReplaceFileW,
+        MOVEFILE_REPLACE_EXISTING, MOVEFILE_WRITE_THROUGH,
+        REPLACEFILE_IGNORE_MERGE_ERRORS,
+    };
+
+    let from_w = HSTRING::from(from.as_os_str());
+    let to_w = HSTRING::from(to.as_os_str());
+
+    // ReplaceFileW preserves ACLs and alternate data streams on the
+    // target, but requires the target to already exist.
+    if to.exists() {
+        let result = unsafe {
+            ReplaceFileW(
+                &to_w,
+                &from_w,
+                windows::core::PCWSTR::null(),
+                REPLACEFILE_IGNORE_MERGE_ERRORS,
+                None,
+                None,
+            )
+        };
+        return result.map_err(|e| io::Error::new(io::ErrorKind::Other, e));
+    }
+
+    // Target doesn't exist yet — fall back to MoveFileExW which handles
+    // both cases but doesn't preserve target metadata (irrelevant here
+    // since there is no target).
+    let result = unsafe {
+        MoveFileExW(
+            &from_w,
+            &to_w,
+            MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH,
+        )
+    };
+    result.map_err(|e| io::Error::new(io::ErrorKind::Other, e))
+}
+
 /// Write `contents` to `path` atomically with `file_mode`, ensuring the
 /// parent directory exists and is set to `dir_mode`.
 ///
@@ -131,6 +173,28 @@ pub async fn write_atomic_restricted(
     .map_err(io::Error::other)?
 }
 
+/// Atomically replace `to` with `from`.
+///
+/// On Unix this is a single `rename(2)` call — atomic by POSIX
+/// guarantee. On Windows this uses `ReplaceFileW` (preserves ACLs
+/// and alternate data streams) when the target exists, falling back
+/// to `MoveFileExW` with `MOVEFILE_REPLACE_EXISTING` for first-write.
+///
+/// # Errors
+///
+/// Returns an error if the rename fails (permission denied, cross-device,
+/// source missing, etc.).
+pub async fn atomic_rename_over(
+    from: impl AsRef<Path>,
+    to: impl AsRef<Path>,
+) -> io::Result<()> {
+    let from = from.as_ref().to_owned();
+    let to = to.as_ref().to_owned();
+    tokio::task::spawn_blocking(move || atomic_rename_over_sync(&from, &to))
+        .await
+        .map_err(io::Error::other)?
+}
+
 /// Remove a file if it exists; silently return `Ok(())` if it does not.
 ///
 /// This is the async counterpart of [`blocking::remove_if_exists`];
@@ -140,17 +204,14 @@ pub async fn write_atomic_restricted(
 ///
 /// Returns an error if `remove_file` fails for any reason other than
 /// `NotFound`.
-pub async fn remove_if_exists(path: impl AsRef<Path>) -> io::Result<()> {
-    let path = path.as_ref().to_owned();
-    tokio::task::spawn_blocking(move || remove_if_exists_sync(&path))
-        .await
-        .map_err(io::Error::other)?
+pub async fn remove_file_if_exists(path: impl AsRef<Path>) -> io::Result<()> {
+    match tokio::fs::remove_file(path).await {
+        Ok(()) => Ok(()),
+        Err(e) if e.kind() == io::ErrorKind::NotFound => Ok(()),
+        Err(e) => Err(e),
+    }
 }
 
-// ---------------------------------------------------------------------------
-// Public blocking module
-// ---------------------------------------------------------------------------
-
 /// Synchronous (blocking) wrappers for callers that cannot use async,
 /// such as extism `host_fn` callbacks.
 pub mod blocking {
@@ -188,10 +249,6 @@ pub mod blocking {
     }
 }
 
-// ---------------------------------------------------------------------------
-// Tests
-// ---------------------------------------------------------------------------
-
 #[cfg(all(test, unix))]
 #[allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)]
 mod tests {
@@ -271,9 +328,36 @@ mod tests {
     async fn async_remove_if_exists() {
         let tmp = tempfile::tempdir().unwrap();
         let target = tmp.path().join("nothing");
-        super::remove_if_exists(&target).await.unwrap();
+        super::remove_file_if_exists(&target).await.unwrap();
         std::fs::write(&target, "x").unwrap();
-        super::remove_if_exists(&target).await.unwrap();
+        super::remove_file_if_exists(&target).await.unwrap();
         assert!(!target.exists());
     }
+
+    #[tokio::test]
+    async fn atomic_rename_over_replaces_target() {
+        let tmp = tempfile::tempdir().unwrap();
+        let src = tmp.path().join("source");
+        let dst = tmp.path().join("target");
+        std::fs::write(&dst, b"old").unwrap();
+        std::fs::write(&src, b"new").unwrap();
+
+        super::atomic_rename_over(&src, &dst).await.unwrap();
+
+        assert_eq!(std::fs::read(&dst).unwrap(), b"new");
+        assert!(!src.exists(), "source should be gone after rename");
+    }
+
+    #[tokio::test]
+    async fn atomic_rename_over_works_when_target_missing() {
+        let tmp = tempfile::tempdir().unwrap();
+        let src = tmp.path().join("source");
+        let dst = tmp.path().join("target");
+        std::fs::write(&src, b"new").unwrap();
+
+        super::atomic_rename_over(&src, &dst).await.unwrap();
+
+        assert_eq!(std::fs::read(&dst).unwrap(), b"new");
+        assert!(!src.exists());
+    }
 }