fix(pi): make shared file commits collision safe

src-tauri/src/pi_config/document.rs

@@ -13,13 +13,14 @@ use jsonc_parser::cst::{
 use jsonc_parser::ParseOptions;
 use regex::Regex;
 use serde_json::Value;
-use sha2::{Digest, Sha256};
 use std::collections::HashMap;
 use std::fs::{self, File, Metadata, OpenOptions};
 use std::io::{Read, Take};
 use std::path::{Path, PathBuf};
 use std::sync::{Arc, LazyLock, Mutex, MutexGuard};
 
+use super::shared_file::compare_exchange_shared_file_bytes;
+
 const MAX_PI_MODELS_BYTES: u64 = 8 * 1024 * 1024;
 const EMPTY_MODELS_DOCUMENT: &str = "{\"providers\":{}}";
 const MAX_MUTATION_ATTEMPTS: usize = 3;
@@ -390,10 +391,6 @@ pub(super) fn read_pi_models_document(path: &Path) -> Result<PiModelsDocument, A
     parse_models_source(path, source)
 }
 
-fn fingerprint(bytes: Option<&[u8]>) -> Option<[u8; 32]> {
-    bytes.map(|bytes| Sha256::digest(bytes).into())
-}
-
 fn serialize_models_mutation(
     path: &Path,
     before: Option<&[u8]>,
@@ -449,7 +446,6 @@ pub(crate) fn apply_pi_provider_patch(
 
     for _ in 0..MAX_MUTATION_ATTEMPTS {
         let before = read_models_bytes(path)?;
-        let observed = fingerprint(before.as_deref());
         let serialized = serialize_models_mutation(path, before.as_deref(), &|document| {
             let providers = document
                 .as_object_mut()
@@ -469,12 +465,18 @@ pub(crate) fn apply_pi_provider_patch(
             Ok(())
         })?;
 
-        let current = read_models_bytes(path)?;
-        if fingerprint(current.as_deref()) != observed {
-            continue;
+        match compare_exchange_shared_file_bytes(
+            path,
+            before.as_deref(),
+            &serialized,
+            MAX_PI_MODELS_BYTES,
+            None,
+            "Pi models file",
+        ) {
+            Ok(_) => return Ok(()),
+            Err(AppError::Conflict(_)) => continue,
+            Err(error) => return Err(error),
         }
-        crate::config::atomic_write(path, &serialized)?;
-        return Ok(());
     }
 
     Err(AppError::Conflict(format!(
@@ -570,6 +572,38 @@ mod tests {
         );
     }
 
+    #[test]
+    fn external_rename_during_patch_is_reparsed_before_owned_fields_change() {
+        let temp = tempfile::tempdir().expect("tempdir");
+        let path = temp.path().join("models.json");
+        fs::write(
+            &path,
+            r#"{"providers":{"managed":{"models":[{"id":"old"}]}}}"#,
+        )
+        .expect("seed");
+        crate::pi_config::shared_file::replace_before_next_compare_exchange(
+            &path,
+            br#"{
+  "externalRevision": 7,
+  "providers": {
+    "native": {"models": [{"id": "external"}]},
+    "managed": {"models": [{"id": "old"}]}
+  }
+}"#,
+        );
+        let patch = IndexMap::from([(
+            "managed".to_string(),
+            Some(serde_json::json!({"models": [{"id": "new"}]})),
+        )]);
+
+        apply_pi_provider_patch(&path, &patch).expect("retry patch");
+
+        let saved: Value = serde_json::from_slice(&fs::read(&path).expect("read")).expect("parse");
+        assert_eq!(saved["externalRevision"], 7);
+        assert_eq!(saved["providers"]["native"]["models"][0]["id"], "external");
+        assert_eq!(saved["providers"]["managed"]["models"][0]["id"], "new");
+    }
+
     #[test]
     fn exact_key_delete_does_not_delete_same_content_sibling() {
         let temp = tempfile::tempdir().expect("tempdir");

src-tauri/src/pi_config/native_settings.rs

@@ -6,12 +6,13 @@
 use crate::error::AppError;
 use serde::{Deserialize, Serialize};
 use serde_json::{Map, Value};
-use sha2::{Digest, Sha256};
 use std::fs::{self, File, Metadata, OpenOptions};
 use std::io::{Read, Take};
 use std::path::{Path, PathBuf};
 use std::sync::{LazyLock, Mutex};
 
+use super::shared_file::compare_exchange_shared_file_bytes;
+
 const MAX_PI_SETTINGS_BYTES: u64 = 1024 * 1024;
 const MAX_WRITE_ATTEMPTS: usize = 3;
 static SETTINGS_WRITE_LOCK: LazyLock<Mutex<()>> = LazyLock::new(|| Mutex::new(()));
@@ -130,7 +131,6 @@ fn mutate_settings_document(
 
     for _ in 0..MAX_WRITE_ATTEMPTS {
         let before = read_regular_bytes(path, MAX_PI_SETTINGS_BYTES)?;
-        let observed = fingerprint(before.as_deref());
         let mut document = match before.as_deref() {
             Some(bytes) => {
                 serde_json::from_slice(bytes).map_err(|error| AppError::json(path, error))?
@@ -148,12 +148,18 @@ fn mutate_settings_document(
             .map_err(|source| AppError::JsonSerialize { source })?;
         serialized.push(b'\n');
 
-        let current = read_regular_bytes(path, MAX_PI_SETTINGS_BYTES)?;
-        if fingerprint(current.as_deref()) != observed {
-            continue;
+        match compare_exchange_shared_file_bytes(
+            path,
+            before.as_deref(),
+            &serialized,
+            MAX_PI_SETTINGS_BYTES,
+            None,
+            "Pi settings",
+        ) {
+            Ok(_) => return Ok(()),
+            Err(AppError::Conflict(_)) => continue,
+            Err(error) => return Err(error),
         }
-        crate::config::atomic_write(path, &serialized)?;
-        return Ok(());
     }
 
     Err(AppError::Conflict(format!(
@@ -169,10 +175,6 @@ fn read_settings_document(path: &Path) -> Result<Value, AppError> {
     }
 }
 
-fn fingerprint(bytes: Option<&[u8]>) -> Option<[u8; 32]> {
-    bytes.map(|bytes| Sha256::digest(bytes).into())
-}
-
 #[cfg(unix)]
 fn open_read_only(path: &Path) -> std::io::Result<File> {
     use std::os::unix::fs::OpenOptionsExt;
@@ -286,6 +288,34 @@ mod tests {
         assert_eq!(saved["defaultModel"], "model");
     }
 
+    #[test]
+    fn external_rename_during_settings_patch_is_reparsed_before_retry() {
+        let temp = tempfile::tempdir().expect("tempdir");
+        let path = temp.path().join("settings.json");
+        fs::write(
+            &path,
+            br#"{"theme":"before","defaultProvider":"old","defaultModel":"old"}"#,
+        )
+        .expect("seed");
+        crate::pi_config::shared_file::replace_before_next_compare_exchange(
+            &path,
+            br#"{"theme":"external","packages":["foreign"],"defaultProvider":"old","defaultModel":"old"}"#,
+        );
+
+        mutate_settings_document(&path, |root| {
+            root.insert("defaultProvider".into(), json!("managed"));
+            root.insert("defaultModel".into(), json!("model"));
+            Ok(())
+        })
+        .expect("retry mutation");
+
+        let saved: Value = serde_json::from_slice(&fs::read(&path).expect("read")).expect("parse");
+        assert_eq!(saved["theme"], "external");
+        assert_eq!(saved["packages"], json!(["foreign"]));
+        assert_eq!(saved["defaultProvider"], "managed");
+        assert_eq!(saved["defaultModel"], "model");
+    }
+
     #[cfg(unix)]
     #[test]
     fn settings_symlink_is_rejected() {

src-tauri/src/pi_config/shared_file.rs

@@ -2,19 +2,28 @@
 //!
 //! Callers choose an exact path and size limit. This layer supplies bounded
 //! regular-file reads, symlink rejection, optimistic revisions, per-path
-//! process locking, durable atomic replacement, and compare-before-delete.
+//! process locking, and OS-backed compare/exchange replacement. The latter is
+//! deliberately stronger than "read, compare, rename": the displaced path is
+//! inspected after one atomic namespace operation, so an external Pi/user
+//! rename in the commit window is restored instead of overwritten.
 
 use crate::error::AppError;
 use sha2::{Digest, Sha256};
 use std::collections::HashMap;
 use std::fs::{self, File, Metadata, OpenOptions};
-use std::io::{Read, Take};
+use std::io::{Read, Take, Write};
 use std::path::{Path, PathBuf};
 use std::sync::{Arc, LazyLock, Mutex};
 
 static FILE_LOCKS: LazyLock<Mutex<HashMap<PathBuf, Arc<Mutex<()>>>>> =
     LazyLock::new(|| Mutex::new(HashMap::new()));
 
+#[cfg(test)]
+type CompareExchangeHooks = HashMap<PathBuf, Vec<u8>>;
+#[cfg(test)]
+static BEFORE_COMPARE_EXCHANGE: LazyLock<Mutex<CompareExchangeHooks>> =
+    LazyLock::new(|| Mutex::new(HashMap::new()));
+
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub(crate) struct SharedFileSnapshot {
     pub revision: String,
@@ -58,8 +67,14 @@ pub(crate) fn replace_shared_file(
         .map_err(|error| AppError::Config(format!("Pi file lock is poisoned: {error}")))?;
     let current = read_shared_file(path, max_bytes, label)?;
     ensure_revision(path, expected_revision, &current.revision)?;
-    crate::config::atomic_write_durable(path, bytes, new_file_mode)?;
-    read_shared_file(path, max_bytes, label)
+    compare_exchange_under_lock(
+        path,
+        current.bytes.as_deref(),
+        Some(bytes),
+        max_bytes,
+        new_file_mode,
+        label,
+    )
 }
 
 pub(crate) fn delete_shared_file(
@@ -77,16 +92,507 @@ pub(crate) fn delete_shared_file(
     if !current.exists() {
         return Ok(false);
     }
-    fs::remove_file(path).map_err(|error| AppError::io(path, error))?;
-    #[cfg(unix)]
-    if let Some(parent) = path.parent() {
-        File::open(parent)
-            .and_then(|directory| directory.sync_all())
-            .map_err(|error| AppError::io(parent, error))?;
-    }
+    compare_exchange_under_lock(path, current.bytes.as_deref(), None, max_bytes, None, label)?;
     Ok(true)
 }
 
+/// Atomically replace the exact bytes a caller parsed.
+///
+/// This is the common commit primitive for shared Pi documents. Callers may
+/// retry `Conflict` after reparsing, but other failures are fail-closed. The
+/// target never gets replaced merely because a pre-rename fingerprint happened
+/// to match.
+pub(crate) fn compare_exchange_shared_file_bytes(
+    path: &Path,
+    expected: Option<&[u8]>,
+    replacement: &[u8],
+    max_bytes: u64,
+    new_file_mode: Option<u32>,
+    label: &str,
+) -> Result<SharedFileSnapshot, AppError> {
+    if replacement.len() as u64 > max_bytes {
+        return Err(AppError::InvalidInput(format!(
+            "{label} exceeds the {max_bytes}-byte limit"
+        )));
+    }
+    let lock = path_lock(path)?;
+    let _guard = lock
+        .lock()
+        .map_err(|error| AppError::Config(format!("Pi file lock is poisoned: {error}")))?;
+    compare_exchange_under_lock(
+        path,
+        expected,
+        Some(replacement),
+        max_bytes,
+        new_file_mode,
+        label,
+    )
+}
+
+fn compare_exchange_under_lock(
+    path: &Path,
+    expected: Option<&[u8]>,
+    replacement: Option<&[u8]>,
+    max_bytes: u64,
+    new_file_mode: Option<u32>,
+    label: &str,
+) -> Result<SharedFileSnapshot, AppError> {
+    if replacement.is_some_and(|bytes| bytes.len() as u64 > max_bytes) {
+        return Err(AppError::InvalidInput(format!(
+            "{label} exceeds the {max_bytes}-byte limit"
+        )));
+    }
+    let parent = path.parent().ok_or_else(|| {
+        AppError::InvalidInput(format!("{label} path has no parent: {}", path.display()))
+    })?;
+    fs::create_dir_all(parent).map_err(|error| AppError::io(parent, error))?;
+
+    // This preflight rejects symlinks/non-regular files and avoids a namespace
+    // operation when the conflict is already visible. Correctness still rests
+    // on inspecting the displaced file after the atomic operation below.
+    let before = read_regular_bytes(path, max_bytes, label)?;
+    if before.as_deref() != expected {
+        return Err(concurrent_change(path, label));
+    }
+
+    let staged = replacement
+        .map(|bytes| stage_replacement(path, bytes, before.is_some(), new_file_mode))
+        .transpose()?;
+    run_before_compare_exchange_hook(path)?;
+
+    let result = match (expected, replacement, staged.as_deref()) {
+        (None, Some(bytes), Some(staged)) => match rename_noreplace(staged, path) {
+            Ok(()) => {
+                sync_parent(parent)?;
+                Ok(snapshot(Some(bytes)))
+            }
+            Err(error) if is_destination_exists(&error) => Err(concurrent_change(path, label)),
+            Err(error) => Err(rename_error("create", staged, path, error)),
+        },
+        (Some(expected), Some(replacement), Some(staged)) => {
+            replace_existing_if_equal(path, staged, expected, replacement, max_bytes, label)
+        }
+        (Some(expected), None, None) => delete_existing_if_equal(path, expected, max_bytes, label),
+        (None, None, None) => Ok(snapshot(None)),
+        _ => Err(AppError::Config(
+            "invalid Pi shared-file compare/exchange state".to_string(),
+        )),
+    };
+
+    if let Some(staged) = staged {
+        // Success consumes or removes the staged path. On a failed rollback it
+        // intentionally remains as a recovery artifact containing data that
+        // must not be discarded, so only remove a file still containing our
+        // proposed replacement.
+        if replacement.is_some_and(|bytes| {
+            read_regular_bytes(&staged, max_bytes, label)
+                .ok()
+                .flatten()
+                .as_deref()
+                == Some(bytes)
+        }) {
+            let _ = fs::remove_file(&staged);
+        }
+    }
+    result
+}
+
+fn replace_existing_if_equal(
+    path: &Path,
+    staged: &Path,
+    expected: &[u8],
+    replacement: &[u8],
+    max_bytes: u64,
+    label: &str,
+) -> Result<SharedFileSnapshot, AppError> {
+    let displaced = match install_over_existing(staged, path) {
+        Ok(displaced) => displaced,
+        Err(error)
+            if matches!(
+                error.kind(),
+                std::io::ErrorKind::NotFound | std::io::ErrorKind::AlreadyExists
+            ) =>
+        {
+            return Err(concurrent_change(path, label));
+        }
+        Err(error) => return Err(rename_error("exchange", staged, path, error)),
+    };
+    let displaced_bytes = read_regular_bytes(&displaced, max_bytes, label);
+    if matches!(
+        displaced_bytes,
+        Ok(ref bytes) if bytes.as_deref() == Some(expected)
+    ) {
+        fs::remove_file(&displaced).map_err(|error| AppError::io(&displaced, error))?;
+        sync_parent(
+            path.parent()
+                .expect("a path accepted by compare/exchange has a parent"),
+        )?;
+        return Ok(snapshot(Some(replacement)));
+    }
+
+    let proposed_or_raced = restore_displaced(&displaced, path).map_err(|error| {
+        AppError::Conflict(format!(
+            "{label} changed during atomic replacement and could not be restored; \
+             the displaced bytes remain at {}: {error}",
+            displaced.display()
+        ))
+    })?;
+    let proposed_bytes = read_regular_bytes(&proposed_or_raced, max_bytes, label)?;
+    if proposed_bytes.as_deref() == Some(replacement) {
+        fs::remove_file(&proposed_or_raced)
+            .map_err(|error| AppError::io(&proposed_or_raced, error))?;
+    }
+    sync_parent(
+        path.parent()
+            .expect("a path accepted by compare/exchange has a parent"),
+    )?;
+    match displaced_bytes {
+        Ok(_) => Err(concurrent_change(path, label)),
+        Err(error) => Err(AppError::Conflict(format!(
+            "{label} became unsafe during atomic replacement and was restored: {error}"
+        ))),
+    }
+}
+
+fn delete_existing_if_equal(
+    path: &Path,
+    expected: &[u8],
+    max_bytes: u64,
+    label: &str,
+) -> Result<SharedFileSnapshot, AppError> {
+    let quarantine = sibling_temp_path(path, "delete");
+    rename_noreplace(path, &quarantine)
+        .map_err(|error| rename_error("quarantine", path, &quarantine, error))?;
+    let quarantined = read_regular_bytes(&quarantine, max_bytes, label);
+    if matches!(
+        quarantined,
+        Ok(ref bytes) if bytes.as_deref() == Some(expected)
+    ) {
+        fs::remove_file(&quarantine).map_err(|error| AppError::io(&quarantine, error))?;
+        sync_parent(
+            path.parent()
+                .expect("a path accepted by compare/exchange has a parent"),
+        )?;
+        return Ok(snapshot(None));
+    }
+
+    match rename_noreplace(&quarantine, path) {
+        Ok(()) => {
+            sync_parent(
+                path.parent()
+                    .expect("a path accepted by compare/exchange has a parent"),
+            )?;
+            match quarantined {
+                Ok(_) => Err(concurrent_change(path, label)),
+                Err(error) => Err(AppError::Conflict(format!(
+                    "{label} became unsafe during delete and was restored: {error}"
+                ))),
+            }
+        }
+        Err(error) => Err(AppError::Conflict(format!(
+            "{label} changed during delete and the displaced bytes remain at {}: {error}",
+            quarantine.display()
+        ))),
+    }
+}
+
+fn stage_replacement(
+    path: &Path,
+    bytes: &[u8],
+    preserve_mode: bool,
+    new_file_mode: Option<u32>,
+) -> Result<PathBuf, AppError> {
+    let staged = sibling_temp_path(path, "cas");
+    let mut options = OpenOptions::new();
+    options.create_new(true).write(true);
+    #[cfg(unix)]
+    {
+        use std::os::unix::fs::{OpenOptionsExt, PermissionsExt};
+        let mode = if preserve_mode {
+            fs::metadata(path)
+                .map(|metadata| metadata.permissions().mode())
+                .unwrap_or_else(|_| new_file_mode.unwrap_or(0o666))
+        } else {
+            new_file_mode.unwrap_or(0o666)
+        };
+        options.mode(mode);
+    }
+    let mut file = options
+        .open(&staged)
+        .map_err(|error| AppError::io(&staged, error))?;
+    file.write_all(bytes)
+        .map_err(|error| AppError::io(&staged, error))?;
+    file.flush()
+        .and_then(|_| file.sync_all())
+        .map_err(|error| AppError::io(&staged, error))?;
+    Ok(staged)
+}
+
+fn sibling_temp_path(path: &Path, purpose: &str) -> PathBuf {
+    let parent = path
+        .parent()
+        .expect("a path accepted by compare/exchange has a parent");
+    let name = path
+        .file_name()
+        .expect("a path accepted by compare/exchange has a file name")
+        .to_string_lossy();
+    parent.join(format!(
+        ".{name}.{purpose}.{}",
+        uuid::Uuid::new_v4().simple()
+    ))
+}
+
+fn snapshot(bytes: Option<&[u8]>) -> SharedFileSnapshot {
+    SharedFileSnapshot {
+        revision: revision(bytes),
+        bytes: bytes.map(ToOwned::to_owned),
+    }
+}
+
+fn concurrent_change(path: &Path, label: &str) -> AppError {
+    AppError::Conflict(format!(
+        "{label} changed since it was read: {}",
+        path.display()
+    ))
+}
+
+fn rename_error(
+    operation: &str,
+    source: &Path,
+    destination: &Path,
+    error: std::io::Error,
+) -> AppError {
+    AppError::IoContext {
+        context: format!(
+            "Pi shared-file {operation} failed: {} -> {}",
+            source.display(),
+            destination.display()
+        ),
+        source: error,
+    }
+}
+
+#[cfg(unix)]
+fn sync_parent(parent: &Path) -> Result<(), AppError> {
+    File::open(parent)
+        .and_then(|directory| directory.sync_all())
+        .map_err(|error| AppError::io(parent, error))
+}
+
+#[cfg(not(unix))]
+fn sync_parent(_parent: &Path) -> Result<(), AppError> {
+    Ok(())
+}
+
+#[cfg(any(target_os = "linux", target_os = "macos"))]
+fn c_path(path: &Path) -> std::io::Result<std::ffi::CString> {
+    use std::os::unix::ffi::OsStrExt;
+    std::ffi::CString::new(path.as_os_str().as_bytes())
+        .map_err(|_| std::io::Error::new(std::io::ErrorKind::InvalidInput, "path contains NUL"))
+}
+
+#[cfg(target_os = "linux")]
+fn rename_noreplace(source: &Path, destination: &Path) -> std::io::Result<()> {
+    let source = c_path(source)?;
+    let destination = c_path(destination)?;
+    // SAFETY: both C strings remain alive and renameat2 performs one
+    // synchronous namespace operation.
+    let result = unsafe {
+        libc::renameat2(
+            libc::AT_FDCWD,
+            source.as_ptr(),
+            libc::AT_FDCWD,
+            destination.as_ptr(),
+            libc::RENAME_NOREPLACE,
+        )
+    };
+    if result == 0 {
+        Ok(())
+    } else {
+        Err(std::io::Error::last_os_error())
+    }
+}
+
+#[cfg(target_os = "linux")]
+fn exchange_paths(left: &Path, right: &Path) -> std::io::Result<()> {
+    let left = c_path(left)?;
+    let right = c_path(right)?;
+    // SAFETY: both C strings remain alive and renameat2 performs one
+    // synchronous namespace operation.
+    let result = unsafe {
+        libc::renameat2(
+            libc::AT_FDCWD,
+            left.as_ptr(),
+            libc::AT_FDCWD,
+            right.as_ptr(),
+            libc::RENAME_EXCHANGE,
+        )
+    };
+    if result == 0 {
+        Ok(())
+    } else {
+        Err(std::io::Error::last_os_error())
+    }
+}
+
+#[cfg(target_os = "macos")]
+fn rename_with_flags(source: &Path, destination: &Path, flags: u32) -> std::io::Result<()> {
+    let source = c_path(source)?;
+    let destination = c_path(destination)?;
+    // SAFETY: both C strings remain alive and renamex_np performs one
+    // synchronous namespace operation.
+    let result = unsafe { libc::renamex_np(source.as_ptr(), destination.as_ptr(), flags) };
+    if result == 0 {
+        Ok(())
+    } else {
+        Err(std::io::Error::last_os_error())
+    }
+}
+
+#[cfg(target_os = "macos")]
+fn rename_noreplace(source: &Path, destination: &Path) -> std::io::Result<()> {
+    rename_with_flags(source, destination, libc::RENAME_EXCL)
+}
+
+#[cfg(target_os = "macos")]
+fn exchange_paths(left: &Path, right: &Path) -> std::io::Result<()> {
+    rename_with_flags(left, right, libc::RENAME_SWAP)
+}
+
+#[cfg(windows)]
+fn wide_path(path: &Path) -> Vec<u16> {
+    use std::os::windows::ffi::OsStrExt;
+    path.as_os_str().encode_wide().chain(Some(0)).collect()
+}
+
+#[cfg(windows)]
+fn rename_noreplace(source: &Path, destination: &Path) -> std::io::Result<()> {
+    use windows_sys::Win32::Storage::FileSystem::{MoveFileExW, MOVEFILE_WRITE_THROUGH};
+    let source = wide_path(source);
+    let destination = wide_path(destination);
+    // SAFETY: both buffers are NUL-terminated and remain alive during the
+    // synchronous Win32 call. No REPLACE_EXISTING flag is supplied.
+    let result = unsafe {
+        MoveFileExW(
+            source.as_ptr(),
+            destination.as_ptr(),
+            MOVEFILE_WRITE_THROUGH,
+        )
+    };
+    if result != 0 {
+        Ok(())
+    } else {
+        Err(std::io::Error::last_os_error())
+    }
+}
+
+#[cfg(windows)]
+fn replace_file_with_backup(path: &Path, replacement: &Path, backup: &Path) -> std::io::Result<()> {
+    use windows_sys::Win32::Storage::FileSystem::{ReplaceFileW, REPLACEFILE_WRITE_THROUGH};
+    let path = wide_path(path);
+    let replacement = wide_path(replacement);
+    let backup = wide_path(backup);
+    // SAFETY: all buffers are NUL-terminated and remain alive during the
+    // synchronous Win32 call.
+    let result = unsafe {
+        ReplaceFileW(
+            path.as_ptr(),
+            replacement.as_ptr(),
+            backup.as_ptr(),
+            REPLACEFILE_WRITE_THROUGH,
+            std::ptr::null(),
+            std::ptr::null(),
+        )
+    };
+    if result != 0 {
+        Ok(())
+    } else {
+        Err(std::io::Error::last_os_error())
+    }
+}
+
+#[cfg(any(target_os = "linux", target_os = "macos"))]
+fn install_over_existing(staged: &Path, path: &Path) -> std::io::Result<PathBuf> {
+    exchange_paths(staged, path)?;
+    Ok(staged.to_path_buf())
+}
+
+#[cfg(windows)]
+fn install_over_existing(staged: &Path, path: &Path) -> std::io::Result<PathBuf> {
+    let backup = sibling_temp_path(path, "displaced");
+    replace_file_with_backup(path, staged, &backup)?;
+    Ok(backup)
+}
+
+#[cfg(any(target_os = "linux", target_os = "macos"))]
+fn restore_displaced(displaced: &Path, path: &Path) -> std::io::Result<PathBuf> {
+    exchange_paths(displaced, path)?;
+    Ok(displaced.to_path_buf())
+}
+
+#[cfg(windows)]
+fn restore_displaced(displaced: &Path, path: &Path) -> std::io::Result<PathBuf> {
+    let backup = sibling_temp_path(path, "rejected");
+    replace_file_with_backup(path, displaced, &backup)?;
+    Ok(backup)
+}
+
+#[cfg(not(any(target_os = "linux", target_os = "macos", windows)))]
+fn rename_noreplace(_source: &Path, _destination: &Path) -> std::io::Result<()> {
+    Err(std::io::Error::new(
+        std::io::ErrorKind::Unsupported,
+        "atomic no-replace rename is unsupported on this platform",
+    ))
+}
+
+#[cfg(not(any(target_os = "linux", target_os = "macos", windows)))]
+fn install_over_existing(_staged: &Path, _path: &Path) -> std::io::Result<PathBuf> {
+    Err(std::io::Error::new(
+        std::io::ErrorKind::Unsupported,
+        "atomic file exchange is unsupported on this platform",
+    ))
+}
+
+#[cfg(not(any(target_os = "linux", target_os = "macos", windows)))]
+fn restore_displaced(_displaced: &Path, _path: &Path) -> std::io::Result<PathBuf> {
+    Err(std::io::Error::new(
+        std::io::ErrorKind::Unsupported,
+        "atomic file exchange is unsupported on this platform",
+    ))
+}
+
+fn is_destination_exists(error: &std::io::Error) -> bool {
+    error.kind() == std::io::ErrorKind::AlreadyExists
+        || matches!(error.raw_os_error(), Some(libc::EEXIST))
+}
+
+#[cfg(test)]
+pub(crate) fn replace_before_next_compare_exchange(path: &Path, bytes: &[u8]) {
+    BEFORE_COMPARE_EXCHANGE
+        .lock()
+        .expect("compare/exchange test hook lock")
+        .insert(path.to_path_buf(), bytes.to_vec());
+}
+
+#[cfg(test)]
+fn run_before_compare_exchange_hook(path: &Path) -> Result<(), AppError> {
+    let replacement = {
+        let mut hook = BEFORE_COMPARE_EXCHANGE
+            .lock()
+            .map_err(|error| AppError::Config(format!("Pi CAS test hook is poisoned: {error}")))?;
+        hook.remove(path)
+    };
+    if let Some(bytes) = replacement {
+        crate::config::atomic_write_durable(path, &bytes, None)?;
+    }
+    Ok(())
+}
+
+#[cfg(not(test))]
+fn run_before_compare_exchange_hook(_path: &Path) -> Result<(), AppError> {
+    Ok(())
+}
+
 fn ensure_revision(path: &Path, expected: &str, actual: &str) -> Result<(), AppError> {
     if expected == actual {
         Ok(())
@@ -176,7 +682,7 @@ fn read_regular_bytes(
     }
     let mut file = open_read_only(path).map_err(|error| AppError::io(path, error))?;
     let opened = file.metadata().map_err(|error| AppError::io(path, error))?;
-    let bytes = read_limited(file.by_ref().take(max_bytes + 1), path, max_bytes)?;
+    let bytes = read_limited(Read::by_ref(&mut file).take(max_bytes + 1), path, max_bytes)?;
     let completed = file.metadata().map_err(|error| AppError::io(path, error))?;
     let current = fs::symlink_metadata(path).map_err(|error| AppError::io(path, error))?;
     if !current.file_type().is_file()
@@ -216,6 +722,31 @@ mod tests {
         assert!(delete_shared_file(&path, &replaced.revision, 1024, "test").expect("delete"));
     }
 
+    #[test]
+    fn external_rename_in_the_commit_window_is_restored_without_data_loss() {
+        let temp = tempfile::tempdir().expect("tempdir");
+        let path = temp.path().join("shared.md");
+        fs::write(&path, b"observed").expect("seed");
+        let observed = read_shared_file(&path, 1024, "test").expect("snapshot");
+
+        replace_before_next_compare_exchange(&path, b"external replacement");
+        let replace = replace_shared_file(&path, &observed.revision, b"ours", 1024, None, "test");
+        assert!(matches!(replace, Err(AppError::Conflict(_))));
+        assert_eq!(
+            fs::read(&path).expect("external bytes restored"),
+            b"external replacement"
+        );
+
+        let observed = read_shared_file(&path, 1024, "test").expect("snapshot");
+        replace_before_next_compare_exchange(&path, b"external before delete");
+        let delete = delete_shared_file(&path, &observed.revision, 1024, "test");
+        assert!(matches!(delete, Err(AppError::Conflict(_))));
+        assert_eq!(
+            fs::read(&path).expect("external bytes restored"),
+            b"external before delete"
+        );
+    }
+
     #[cfg(unix)]
     #[test]
     fn symlink_targets_fail_closed() {