Fix: [LinkSafeFileSystem] stop links from shadowing real folders in EnumerateFiles - #1743
NeuralFault wants to merge 2 commits into
Conversation
…umerateFiles - Key real directories ordinally and claim their identity on pop, not push, so case-distinct folders are both scanned and scan order decides the winner - Drain real directories before following any links so a link (e.g. `diffusion_models` -> `DiffusionModels`) can never take the visited key and drop the real folder's subtree - Compare link-resolved targets with platform case sensitivity (PathComparer) against real dirs and other links to keep the loop guards intact - Log skipped links at Info and skipped real directories at Warn, naming the earlier scanned path - Add tests for sibling and deeper shadow links, plus a Windows-only junction target case-mismatch test
|
Nice catch, and the fix is right — claim-on-pop + draining real dirs before links closes the shadowing properly. All 16 tests pass on Windows here too. One regression I could reproduce: the non-link branch checks Repro: root holds One condition fixes it (all tests + the repro pass with it): if (
visitedRealDirsExact.TryGetValue(dir.RealPath, out var realClaimer)
|| visitedLinkTargets.TryGetValue(dir.RealPath, out realClaimer)
)Real dirs under the root are all claimed before any link pops, so this can't reintroduce the shadowing. Would be great to add that nested-targets case as a test (both name orders as DataRows). Non-blocking nits:
🤖 Reviewed with Claude Code on mohnjiles' behalf |
… targets A real directory reached through a link was only checked against the real directory keys, so a link that had already claimed the same physical directory did not dedupe it, and the files were yielded twice. - Check visitedLinkTargets in the real directory branch as well - Drop the redundant IsLink stack field and trim the EnumerateFiles summary - Add a depth-based regression test that pins walk order portably, plus a Windows-only test covering the reported sibling-link repro
|
Yep, reproduced. The real dir branch was only checking The name-order test you suggested only decides the walk order on NTFS, and the suite runs on Linux here on my end where it doesn't, so both rows passed even before the fix. I pinned the order with depth instead, link to Nits done, Also ran it on Windows against the real indexer, not just the unit tests. Then put four junctions in that models dir. The aliased All four deduped and logged the skip, each naming the path that got there first. The nested-target fixture is the only one of the four that needed the new check, the other three were already handled by existing logic. Index came out with 3 entries for the 3 files. |
LinkSafeFileSystem.EnumerateFileshad a regression where a directory link (symlink/junction) pointing at a real folder in the same tree could drop that real folder's entire subtree from enumeration. This left models indexed only under an alias path whose folder name parses toSharedFolderType.Unknown, so the UI showed bare filenames with no preview, version line, or base-model chip, and sidecar metadata was ignored.Problem
1. Link and real folder collided on the visited key
EnumerateFilesvisited each physical directory at most once and keyed identity by resolved path. A link pointing at a second folder in the same tree produced the same key as that folder. Real-world case: adiffusion_modelssymlink sitting beside the realDiffusionModelsfolder (some packages require that folder name).GetRealPath("...\diffusion_models")resolves to...\DiffusionModels, so both entries mapped to one key and one of them had its subtree skipped.2. The loser was decided by reversed push order
The key was claimed inside the reversed push loop (
for i = count-1; i >= 0; i--) while the stack popped in enumeration order, so the last enumerated entry won the key. On NTFS,_(0x5F) sorts aboveZ(0x5A), sodiffusion_modelsalways beatDiffusionModelsand the real folder was dropped.Solution
LinkSafeFileSystem.EnumerateFiles(rewritten)Real directories are now keyed ordinally, so two folders whose names differ only in case are never conflated:
Identity is claimed when a directory is popped, not when it is queued, so scan order decides which spelling wins instead of reversed push order:
Real subdirectories are drained fully before any link is considered via two stacks, so a link can never take the visited key from the folder it points at, including a deeper link pointing at a top-level folder:
Link-resolved targets are compared with platform case sensitivity (
PathComparer), keeping the loop guard correct per OS:PathComparer=OrdinalIgnoreCase): a junction whose stored target is spelled differently from the scan root still matches and is skipped, so the loop guard holds.PathComparer=Ordinal): a link tofoois not conflated with a realFooon a case-sensitive filesystem.Skip logging now distinguishes the benign case (a link shadowed by a real folder, logged at
Info) from data loss (a real folder shadowed, logged atWarn), and names the earlier scanned path:LinkSafeFileSystemTests(added)EnumerateFiles_RealFolderShadowedByLink_KeepsRealFolderPaths:[DataTestMethod]covering both a sibling link (diffusion_models) and a deeper link (sub/alias) pointing atDiffusionModels, asserting only the real-folder paths are yielded.EnumerateFiles_JunctionTargetCaseMismatch_KeepsRealFolderPaths: Windows-only (Compat.IsWindowsguard,Assert.Inconclusiveelsewhere) covering a junction whose stored target casing differs from the scan root.Verification
diffusion_modelsalias no longer hidesDiffusionModels.LinkSafeFileSystemTestsgives 15 passed, 0 failed (1 skipped: the Windows-only junction test). Could not confirm issue on Linux as particular issue with symlink did not appear. Indexing ofDiffusionModelsresolved properly withdiffusion_modelssymlink present.dotnet buildforStabilityMatrix.Coresucceeds with 0 errors.Issue diagnosed with assistance from Hermes Agent / DeepSeek V4.1 Flash using local repo reference and logs provided by reporting Discord user.
Reported issue reproduced, and the fix confirmed post-change.
Authored with assistance from DeepSeek V4 Pro