Fix: [LinkSafeFileSystem] stop links from shadowing real folders in EnumerateFiles

StabilityMatrix.Core/Helper/LinkSafeFileSystem.cs

@@ -98,26 +98,79 @@ public static bool WouldLinkCycle(DirectoryPath sourceDir, DirectoryPath linkPat
 
     /// <summary>
     /// Recursively enumerates files matching <paramref name="searchPattern"/> under
-    /// <paramref name="rootDir"/>. Linked directories are followed once; a link back to a directory
-    /// already visited is skipped, as are directories deeper than <paramref name="maxDepth"/>.
-    /// Inaccessible directories are skipped rather than aborting the enumeration.
-    /// Yielded paths are rooted at <paramref name="rootDir"/> as given, not at its resolved target.
+    /// <paramref name="rootDir"/>. Symbolic links are followed, but every physical directory is
+    /// visited at most once, so no file is yielded twice. Directories nested deeper than
+    /// <paramref name="maxDepth"/> and directories that cannot be read are skipped without aborting
+    /// the enumeration. Yielded paths are rooted at <paramref name="rootDir"/> as given, not at its
+    /// resolved target.
     /// </summary>
     public static IEnumerable<string> EnumerateFiles(
         string rootDir,
         string searchPattern,
         int maxDepth = DefaultMaxDepth
     )
     {
-        var visited = new HashSet<string>(PathComparer);
-        var pending = new Stack<(string Path, string RealPath, int Depth)>();
+        // A real directory is keyed by its literal path, compared ordinally, so two folders whose
+        // names differ only in case are both scanned; it is also matched against link targets, so a
+        // real folder reached through a link is not rescanned. A link is keyed by its resolved
+        // target, compared with the platform's case sensitivity (PathComparer), because a target is
+        // stored however the link was created.
+        var visitedRealDirsExact = new Dictionary<string, string>(StringComparer.Ordinal);
+        var visitedRealDirsForLinkTargets = new Dictionary<string, string>(PathComparer);
+        var visitedLinkTargets = new Dictionary<string, string>(PathComparer);
+
+        // Real directories are drained to completion before any link is considered, so a link can
+        // never take the identity of a real folder and shadow it out of the scan.
+        var realDirs = new Stack<(string Path, string RealPath, int Depth)>();
+        var linkedDirs = new Stack<(string Path, string RealPath, int Depth)>();
 
         var rootReal = GetRealPath(rootDir);
-        visited.Add(rootReal);
-        pending.Push((rootDir, rootReal, 0));
+        realDirs.Push((rootDir, rootReal, 0));
 
-        while (pending.TryPop(out var dir))
+        while (realDirs.Count > 0 || linkedDirs.Count > 0)
         {
+            // Which stack the entry came from is how the walk knows whether it is a link.
+            var fromRealDirs = realDirs.Count > 0;
+            var dir = fromRealDirs ? realDirs.Pop() : linkedDirs.Pop();
+
+            // Claimed on pop, not on push, so the walk order decides which spelling owns the
+            // identity instead of the reversed push order.
+            if (!fromRealDirs)
+            {
+                if (
+                    visitedLinkTargets.TryGetValue(dir.RealPath, out var linkClaimer)
+                    || visitedRealDirsForLinkTargets.TryGetValue(dir.RealPath, out linkClaimer)
+                )
+                {
+                    Logger.Info(
+                        "Skipping {Path}: the same directory was already scanned as {ClaimedBy}",
+                        dir.Path,
+                        linkClaimer
+                    );
+                    continue;
+                }
+
+                visitedLinkTargets[dir.RealPath] = dir.Path;
+            }
+            else
+            {
+                if (
+                    visitedRealDirsExact.TryGetValue(dir.RealPath, out var realClaimer)
+                    || visitedLinkTargets.TryGetValue(dir.RealPath, out realClaimer)
+                )
+                {
+                    Logger.Warn(
+                        "Skipping {Path}: the same directory was already scanned as {ClaimedBy}",
+                        dir.Path,
+                        realClaimer
+                    );
+                    continue;
+                }
+
+                visitedRealDirsExact[dir.RealPath] = dir.Path;
+                visitedRealDirsForLinkTargets[dir.RealPath] = dir.Path;
+            }
+
             List<string> files;
             List<DirectoryInfo> subDirs;
             try
@@ -150,21 +203,21 @@ public static IEnumerable<string> EnumerateFiles(
                 continue;
             }
 
-            // Pushed in reverse so the stack pops them in enumeration order
+            // Pushed in reverse so each stack pops its entries in enumeration order
             for (var i = subDirs.Count - 1; i >= 0; i--)
             {
                 var subDir = subDirs[i];
-                var subReal = subDir.Attributes.HasFlag(FileAttributes.ReparsePoint)
-                    ? GetRealPath(subDir.FullName)
-                    : Path.Join(dir.RealPath, subDir.Name);
+                var isLinkDir = subDir.Attributes.HasFlag(FileAttributes.ReparsePoint);
+                var subReal = isLinkDir ? GetRealPath(subDir.FullName) : Path.Join(dir.RealPath, subDir.Name);
 
-                if (!visited.Add(subReal))
+                if (isLinkDir)
                 {
-                    Logger.Debug("Skipping {Path}: already visited as {RealPath}", subDir.FullName, subReal);
-                    continue;
+                    linkedDirs.Push((subDir.FullName, subReal, dir.Depth + 1));
+                }
+                else
+                {
+                    realDirs.Push((subDir.FullName, subReal, dir.Depth + 1));
                 }
-
-                pending.Push((subDir.FullName, subReal, dir.Depth + 1));
             }
         }
     }

StabilityMatrix.Tests/Helper/LinkSafeFileSystemTests.cs

@@ -100,6 +100,127 @@ public void EnumerateFiles_TwoLinksToSameDirectory_VisitsItOnce()
         Assert.AreEqual(1, files.Count);
     }
 
+    [DataTestMethod]
+    [DataRow(true)]
+    [DataRow(false)]
+    public void EnumerateFiles_RealDirAlreadyClaimedAsLinkTarget_IsVisitedOnce(bool xyLinkIsDeeper)
+    {
+        // ext/X/Y/y.json is reachable two ways: through a link to ext/X/Y, and as a real subfolder
+        // of a link to ext/X. Sibling enumeration order is file-system dependent, so nesting the
+        // links at different depths pins the walk order instead of relying on names: the deeper
+        // link is always popped first (draining real dirs first pushes it last, and the link stack
+        // pops last-in first-out).
+        var root = CreateDir("root");
+        var sub = CreateDir("root", "sub");
+        CreateFile("ext", "X", "Y", "y.json");
+
+        var x = Path.Combine(tempDir, "ext", "X");
+        var xy = Path.Combine(x, "Y");
+
+        // Deep link -> ext/X/Y, shallow link -> ext/X. When the deep link is the one targeting
+        // ext/X/Y, it is walked first and the shallower ext/X link then reaches that same real
+        // folder again through its "Y" child.
+        var xyLink = Path.Combine(xyLinkIsDeeper ? sub : root, "inner");
+        var xLink = Path.Combine(xyLinkIsDeeper ? root : sub, "outer");
+        TempFiles.CreateDirectoryLink(xyLink, xy);
+        TempFiles.CreateDirectoryLink(xLink, x);
+
+        var files = LinkSafeFileSystem.EnumerateFiles(root, "*.json").ToList();
+
+        Assert.AreEqual(1, files.Count, $"Expected one file, got: {string.Join(", ", files)}");
+    }
+
+    [DataTestMethod]
+    [DataRow("a_inner", "b_outer")]
+    [DataRow("b_inner", "a_outer")]
+    public void EnumerateFiles_NestedLinkTarget_SiblingLinkOrder_IsVisitedOnce(
+        string innerName,
+        string outerName
+    )
+    {
+        if (!Compat.IsWindows)
+        {
+            Assert.Inconclusive(
+                "Needs NTFS, which enumerates sibling directories in stored name order; "
+                    + "EnumerateFiles_RealDirAlreadyClaimedAsLinkTarget_IsVisitedOnce covers the same "
+                    + "bug portably by varying depth instead."
+            );
+            return;
+        }
+
+        // The maintainer's original repro: innerName -> ext/X/Y, outerName -> ext/X, so the inner
+        // link's target is also reached as a real subfolder of the outer link. NTFS yields siblings
+        // in name order, so the two rows walk the links in opposite orders.
+        var root = CreateDir("root");
+        CreateFile("ext", "X", "Y", "y.json");
+
+        var x = Path.Combine(tempDir, "ext", "X");
+        var xy = Path.Combine(x, "Y");
+
+        TempFiles.CreateDirectoryLink(Path.Combine(root, innerName), xy);
+        TempFiles.CreateDirectoryLink(Path.Combine(root, outerName), x);
+
+        var files = LinkSafeFileSystem.EnumerateFiles(root, "*.json").ToList();
+
+        Assert.AreEqual(1, files.Count, $"Expected one file, got: {string.Join(", ", files)}");
+    }
+
+    [DataTestMethod]
+    [DataRow("diffusion_models")]
+    [DataRow("sub", "alias")]
+    public void EnumerateFiles_RealFolderShadowedByLink_KeepsRealFolderPaths(params string[] linkSegments)
+    {
+        var root = CreateDir("root");
+        CreateFile("root", "DiffusionModels", "a.json");
+        CreateFile("root", "DiffusionModels", "b.json");
+
+        var linkPath = Path.Combine([root, .. linkSegments]);
+        Directory.CreateDirectory(Path.GetDirectoryName(linkPath)!);
+        TempFiles.CreateDirectoryLink(linkPath, Path.Combine(root, "DiffusionModels"));
+
+        var files = LinkSafeFileSystem.EnumerateFiles(root, "*.json").ToList();
+
+        CollectionAssert.AreEquivalent(
+            new[]
+            {
+                Path.Combine(root, "DiffusionModels", "a.json"),
+                Path.Combine(root, "DiffusionModels", "b.json"),
+            },
+            files
+        );
+    }
+
+    [TestMethod]
+    public void EnumerateFiles_JunctionTargetCaseMismatch_KeepsRealFolderPaths()
+    {
+        if (!Compat.IsWindows)
+        {
+            Assert.Inconclusive("Junctions with a differently-cased stored target are Windows-only.");
+            return;
+        }
+
+        var root = CreateDir("root");
+        CreateFile("root", "DiffusionModels", "a.json");
+        CreateFile("root", "DiffusionModels", "b.json");
+
+        // Store the junction target with different casing than the real folder on disk.
+        TempFiles.CreateDirectoryLink(
+            Path.Combine(root, "diffusion_models"),
+            Path.Combine(root.ToUpperInvariant(), "DIFFUSIONMODELS")
+        );
+
+        var files = LinkSafeFileSystem.EnumerateFiles(root, "*.json").ToList();
+
+        CollectionAssert.AreEquivalent(
+            new[]
+            {
+                Path.Combine(root, "DiffusionModels", "a.json"),
+                Path.Combine(root, "DiffusionModels", "b.json"),
+            },
+            files
+        );
+    }
+
     [TestMethod]
     public void EnumerateFiles_DeeperThanMaxDepth_IsSkipped()
     {