fix(config): isolate atomic writes and preserve long filenames

Create each atomic write through its own exclusively created temporary sibling and keep its open file handle until writing finishes. Concurrent writers publish complete files without truncating or following an existing temporary entry; failed writes or renames clean up their own staging file.

Keep the temporary basename bounded independently of the destination name so previously supported long project filenames still save. Retain the concurrent-writer and symlink regressions and add a 251-byte filename overwrite regression that checks exact bytes and staging cleanup.

Fixes #1233

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

crates/engine/src/config.rs

@@ -164,32 +164,50 @@ impl ConfigStore for DirConfig {
     }
 }
 
-/// The temporary file an atomic write of `path` goes through.
-pub fn temp_path(path: &Path) -> PathBuf {
-    let mut name = path.file_name().map(|n| n.to_os_string()).unwrap_or_default();
-    name.push(".tmp");
-    path.with_file_name(name)
+/// A new, empty temporary file next to `path` for an atomic write of it, and its path. The name
+/// is unique to this process and this write (`.effectcraft-<pid>-<n>.tmp`) and the file is created
+/// exclusively, so two writers never share one and nothing already at that name (a file, a
+/// symbolic link) is opened or followed (#1233). It ends in `.tmp`, so a file left by a hard
+/// kill never counts as an auto-save. Its bounded name does not include the destination's name,
+/// so a valid long destination name still fits the filesystem's component limit.
+fn create_temp(path: &Path) -> std::io::Result<(std::fs::File, PathBuf)> {
+    use std::sync::atomic::{AtomicU64, Ordering};
+    static NEXT: AtomicU64 = AtomicU64::new(0);
+    let mut last = None;
+    for _ in 0..16 {
+        let n = NEXT.fetch_add(1, Ordering::Relaxed);
+        let tmp = path.with_file_name(format!(".effectcraft-{}-{n}.tmp", std::process::id()));
+        match std::fs::OpenOptions::new().write(true).create_new(true).open(&tmp) {
+            Ok(f) => return Ok((f, tmp)),
+            Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => last = Some(e),
+            Err(e) => return Err(e),
+        }
+    }
+    Err(last.unwrap_or_else(|| std::io::Error::other("no free temporary file name")))
 }
 
 /// Write `data` to `path` atomically: write a temporary file next to it, flush it to disk, then
 /// rename it over `path`. A crash at any point leaves either the old file or the new one, never a
-/// torn file.
+/// torn file, and concurrent writers each publish a whole file of their own.
 pub fn atomic_write(path: &Path, data: &[u8]) -> std::io::Result<()> {
-    atomic_write_with(path, data, |tmp, data| {
+    atomic_write_with(path, data, |f, data| {
         use std::io::Write;
-        let mut f = std::fs::File::create(tmp)?;
         f.write_all(data)?;
         f.sync_all()
     })
 }
 
-/// [`atomic_write`] with an injectable temp-file writer (tests simulate a crash mid-write). A
-/// failed write removes the temporary file and leaves `path` untouched.
-pub fn atomic_write_with(path: &Path, data: &[u8], write: impl FnOnce(&Path, &[u8]) -> std::io::Result<()>) -> std::io::Result<()> {
-    let tmp = temp_path(path);
-    if let Err(e) = write(&tmp, data) {
+/// [`atomic_write`] with an injectable temp-file writer (tests simulate a crash mid-write); it is
+/// given the temporary file, already created. A failed write or rename removes the temporary
+/// file and leaves `path` untouched.
+pub fn atomic_write_with(path: &Path, data: &[u8], write: impl FnOnce(&mut std::fs::File, &[u8]) -> std::io::Result<()>) -> std::io::Result<()> {
+    let (mut f, tmp) = create_temp(path)?;
+    let written = write(&mut f, data);
+    // Closed before the rename (Windows can't rename an open file).
+    drop(f);
+    if let Err(e) = written.and_then(|()| std::fs::rename(&tmp, path)) {
         let _ = std::fs::remove_file(&tmp);
         return Err(e);
     }
-    std::fs::rename(&tmp, path)
+    Ok(())
 }

crates/engine/src/tests_settings.rs

@@ -5,7 +5,7 @@ use std::sync::Arc;
 
 use serde_json::json;
 
-use crate::config::{ConfigStore, DirConfig, MemoryConfig, atomic_write, atomic_write_with, temp_path};
+use crate::config::{ConfigStore, DirConfig, MemoryConfig, atomic_write, atomic_write_with};
 use crate::prefs::{PREFS_FILE, Prefs};
 use crate::shortcuts::{DEFAULT_PRESET, Keymaps, normalize};
 use crate::{Session, autosave};
@@ -548,6 +548,66 @@ fn frontend_commands_are_bindable_with_panel_scope() {
 
 // ---------------------------------------------------------------- auto-save / recovery
 
+/// The names in `d`, sorted.
+fn names(d: &std::path::Path) -> Vec<String> {
+    let mut v: Vec<String> = std::fs::read_dir(d).unwrap().map(|e| e.unwrap().file_name().to_string_lossy().into_owned()).collect();
+    v.sort();
+    v
+}
+
+#[cfg(unix)]
+#[test]
+fn atomic_write_overwrites_a_long_valid_filename() {
+    let d = tmp("atomic-long-name");
+    // The old `<name>.tmp` fits a 255-byte component limit for this 251-byte project name.
+    // Including the full name plus a process id and counter made an otherwise valid save fail.
+    let name = format!("{}.ecproj", "x".repeat(244));
+    assert_eq!(name.len(), 251);
+    let f = d.join(&name);
+    std::fs::write(&f, b"original project").unwrap();
+    let replacement = b"replacement project\0bytes\xff";
+    atomic_write(&f, replacement).unwrap();
+    assert_eq!(std::fs::read(&f).unwrap(), replacement);
+    assert_eq!(names(&d), [name], "no temporary file is left after overwriting the project");
+    let _ = std::fs::remove_dir_all(d);
+}
+
+#[test]
+fn concurrent_atomic_writes_each_publish_a_whole_file() {
+    // Writers of the same file used to share one `.tmp`: they truncated each other's data and
+    // the later rename failed once the first had moved it away (#1233).
+    let d = tmp("atomic-concurrent");
+    let f = d.join("shared.ecproj");
+    for round in 0..10 {
+        let contents: Vec<Vec<u8>> = (0..8u8).map(|w| vec![b'a' + w; 256 * 1024 + usize::from(w) * 1000]).collect();
+        std::thread::scope(|sc| {
+            for c in &contents {
+                let f = &f;
+                sc.spawn(move || atomic_write(f, c).expect("every writer succeeds"));
+            }
+        });
+        let got = std::fs::read(&f).unwrap();
+        assert!(contents.contains(&got), "round {round}: the file is one writer's whole data ({} bytes)", got.len());
+        assert_eq!(names(&d), ["shared.ecproj"], "round {round}: no temporary file is left");
+    }
+    let _ = std::fs::remove_dir_all(d);
+}
+
+#[cfg(unix)]
+#[test]
+fn atomic_write_does_not_follow_a_link_at_the_old_temp_name() {
+    let d = tmp("atomic-link");
+    let f = d.join("p.ecproj");
+    let victim = d.join("victim.txt");
+    std::fs::write(&victim, b"keep").unwrap();
+    // The temporary name used to be `<name>.tmp`, opened with File::create (following links).
+    std::os::unix::fs::symlink(&victim, d.join("p.ecproj.tmp")).unwrap();
+    atomic_write(&f, b"new project").unwrap();
+    assert_eq!(std::fs::read(&victim).unwrap(), b"keep", "the link's target is untouched");
+    assert_eq!(std::fs::read(&f).unwrap(), b"new project");
+    let _ = std::fs::remove_dir_all(d);
+}
+
 #[test]
 fn atomic_write_survives_a_crash_mid_write() {
     let d = tmp("atomic");
@@ -555,12 +615,12 @@ fn atomic_write_survives_a_crash_mid_write() {
     atomic_write(&f, b"version 1").unwrap();
     // The writer dies halfway through the temporary file.
     let r = atomic_write_with(&f, b"version 2 is much longer", |tmp, data| {
-        std::fs::write(tmp, &data[..5])?;
+        std::io::Write::write_all(tmp, &data[..5])?;
         Err(std::io::Error::other("power cut"))
     });
     assert!(r.is_err());
     assert_eq!(std::fs::read(&f).unwrap(), b"version 1");
-    assert!(!temp_path(&f).exists(), "the partial temp file is cleaned up");
+    assert_eq!(names(&d), ["p.ecproj"], "the partial temp file is cleaned up");
     // A temp file left by a hard kill doesn't count as an auto-save.
     std::fs::write(d.join("p auto-save 1.ecproj.tmp"), b"junk").unwrap();
     assert!(autosave::existing(&d, "p").is_empty());