From 9688224cb4fb7fa862649104c8e7d8d464c0f659 Mon Sep 17 00:00:00 2001
From: Joe <joe.melsha@live.com>
Date: Tue, 10 Feb 2026 04:10:00 -0600
Subject: [PATCH] Enhance slugification process and deduplication logic

- Updated the `slugify_name` function to strip trailing spaces and dots before checking for Windows reserved names, ensuring correct handling of inputs like "CON.".
- Added tests to validate the new slugification behavior, particularly for Windows reserved names with trailing characters.
- Refactored `syncback_dir_no_meta` to pre-seed `taken_names` from old children's slugified instance names, improving deduplication against existing siblings regardless of iteration order.
---
 src/snapshot_middleware/dir.rs | 27 ++++++++++++++++++++++-----
 src/syncback/file_names.rs     | 27 ++++++++++++++++++++++-----
 2 files changed, 44 insertions(+), 10 deletions(-)

diff --git a/src/snapshot_middleware/dir.rs b/src/snapshot_middleware/dir.rs
index 2ab10ba7..479fc155 100644
--- a/src/snapshot_middleware/dir.rs
+++ b/src/snapshot_middleware/dir.rs
@@ -8,7 +8,10 @@ use memofs::{DirEntry, Vfs};
 
 use crate::{
     snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot, InstigatingSource},
-    syncback::{hash_instance, FsSnapshot, SyncbackReturn, SyncbackSnapshot},
+    syncback::{
+        hash_instance, name_needs_slugify, slugify_name, FsSnapshot, SyncbackReturn,
+        SyncbackSnapshot,
+    },
 };
 
 use super::{meta_file::DirectoryMetadata, snapshot_from_vfs};
@@ -139,10 +142,9 @@ pub fn syncback_dir_no_meta<'sync>(
     let mut removed_children = Vec::new();
 
     // taken_names tracks claimed bare slugs (without extensions) for dedup.
-    // It is NOT seeded from disk filenames — those contain extensions that
-    // would cause mismatches with bare-slug comparisons. Instead, old
-    // children's dedup_keys are accumulated as they are processed in the
-    // loop below (via name_for_inst's third return value).
+    // Pre-seeded from old tree children's slugified instance names so that
+    // new-only children correctly dedup against existing siblings, regardless
+    // of iteration order (see plan: fix_stem-level_dedup §2).
     let mut taken_names: HashSet<String> = HashSet::new();
 
     // Detect duplicate child names (case-insensitive for file system safety).
@@ -184,6 +186,21 @@ pub fn syncback_dir_no_meta<'sync>(
             old_child_map.insert(inst.name(), inst);
         }
 
+        // Pre-seed taken_names from old children's slugified instance names.
+        // This ensures new-only children that happen to slugify to the same
+        // bare slug as an existing sibling will be deduplicated (e.g., new
+        // "A/B" → slug "A_B" won't collide with existing "A_B").
+        // Matches the approach used in api.rs (lines 902-920).
+        for (_, inst) in &old_child_map {
+            let name = inst.name();
+            let slug = if name_needs_slugify(name) {
+                slugify_name(name).to_lowercase()
+            } else {
+                name.to_lowercase()
+            };
+            taken_names.insert(slug);
+        }
+
         for new_child_ref in new_inst.children() {
             let new_child = snapshot.get_new_instance(*new_child_ref).unwrap();
 
diff --git a/src/syncback/file_names.rs b/src/syncback/file_names.rs
index 53c1c623..ba892b6b 100644
--- a/src/syncback/file_names.rs
+++ b/src/syncback/file_names.rs
@@ -243,6 +243,13 @@ pub fn slugify_name(name: &str) -> String {
         }
     }
 
+    // Strip trailing spaces and dots (invalid on Windows) BEFORE the
+    // reserved-name check so that inputs like "CON." or "CON " are
+    // reduced to "CON" and then correctly caught below.
+    while result.ends_with(' ') || result.ends_with('.') {
+        result.pop();
+    }
+
     // Handle Windows reserved names by appending an underscore
     let result_lower = result.to_lowercase();
     for forbidden in INVALID_WINDOWS_NAMES {
@@ -252,11 +259,6 @@ pub fn slugify_name(name: &str) -> String {
         }
     }
 
-    // Strip trailing spaces and dots (invalid on Windows)
-    while result.ends_with(' ') || result.ends_with('.') {
-        result.pop();
-    }
-
     // If the result is empty or all underscores, use a fallback
     if result.is_empty() || result.chars().all(|c| c == '_') {
         result = "instance".to_string();
@@ -381,6 +383,21 @@ mod tests {
         assert_eq!(slugify_name("nul"), "nul_");
     }
 
+    #[test]
+    fn slugify_windows_reserved_with_trailing_dots_and_spaces() {
+        // Trailing dots/spaces must be stripped BEFORE the reserved name
+        // check, otherwise "CON." bypasses the check and becomes "CON".
+        assert_eq!(slugify_name("CON."), "CON_");
+        assert_eq!(slugify_name("CON "), "CON_");
+        assert_eq!(slugify_name("CON.."), "CON_");
+        assert_eq!(slugify_name("CON. "), "CON_");
+        assert_eq!(slugify_name("PRN."), "PRN_");
+        assert_eq!(slugify_name("AUX "), "AUX_");
+        assert_eq!(slugify_name("nul."), "nul_");
+        assert_eq!(slugify_name("com1."), "com1_");
+        assert_eq!(slugify_name("LPT1 "), "LPT1_");
+    }
+
     #[test]
     fn slugify_trailing_dot() {
         // Trailing dots are stripped (invalid on Windows)