From 24fdf7a1b67ca47681e61d5b496e4e36c7f89adf Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 17:29:57 +0000 Subject: [PATCH 1/2] Keep a cloned repository's folder when an owner scan lists it [patch] Scan GitHub Owners replaced every listed repository with one at //, even when Scan Dev Dir had already found it cloned elsewhere, and the old ClonedRepos entry was never visited again. The repository then fetched a folder that does not exist every minute and offered Pull, Commit and Push that all failed. The owner scan now keeps a known repository's path when it is a clone, and UpdateClonedStatus prunes ClonedRepos entries whose repository no longer points at their path. Fixes #438 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01KLpJqJzDAe9WoXTVLt65CJ --- ProjectDirector.Test/OwnerSyncTests.cs | 156 +++++++++++++++++++++++++ ProjectDirector/ProjectDirector.cs | 67 ++++++++++- 2 files changed, 222 insertions(+), 1 deletion(-) create mode 100644 ProjectDirector.Test/OwnerSyncTests.cs diff --git a/ProjectDirector.Test/OwnerSyncTests.cs b/ProjectDirector.Test/OwnerSyncTests.cs new file mode 100644 index 0000000..4da3bca --- /dev/null +++ b/ProjectDirector.Test/OwnerSyncTests.cs @@ -0,0 +1,156 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.ProjectDirector.Test; + +using System; +using System.Collections.Generic; +using System.IO; + +using Microsoft.VisualStudio.TestTools.UnitTesting; + +/// +/// Covers an owner scan meeting a repository that Scan Dev Dir already found cloned elsewhere. +/// +/// +/// Scan Dev Dir adds each clone's owner to the owners it scans, so the documented flow is Scan Dev +/// Dir followed by Scan GitHub Owners. An owner scan that moved such a repository to +/// <dev>/<owner>/<repo> pointed it at a folder that does not exist and left +/// the old clone recorded, so the repository fetched nothing and offered git actions that all failed. +/// +[TestClass] +public sealed class OwnerSyncTests +{ + private static FullyQualifiedGitHubRepoName Name(string repoName) => + FullyQualifiedGitHubRepoName.Create($"ktsu-dev.{repoName}"); + + private static FullyQualifiedLocalRepoPath LocalPath(string path) => + FullyQualifiedLocalRepoPath.Create(path); + + private static GitHubRepository Repository(string localPath, string repoName) => new() + { + OwnerName = GitHubOwnerName.Create("ktsu-dev"), + RepoName = GitHubRepoName.Create(repoName), + LocalPath = LocalPath(localPath), + }; + + private static string CreateDevDirectory() => + Directory.CreateDirectory(Path.Combine(Path.GetTempPath(), $"ktsu_pd_{Guid.NewGuid():N}")).FullName; + + [TestMethod] + public void AnOwnerScanKeepsTheFolderARepositoryIsAlreadyClonedIn() + { + string dev = CreateDevDirectory(); + try + { + string clone = Path.Combine(dev, "ProjectDirector"); + Assert.IsTrue(GitCli.Run("init", clone).Succeeded, "git init failed."); + Dictionary repos = new() + { + [Name("ProjectDirector")] = Repository(clone, "ProjectDirector"), + }; + + FullyQualifiedLocalRepoPath chosen = ProjectDirector.ChooseSyncedLocalPath( + repos, Name("ProjectDirector"), LocalPath(Path.Combine(dev, "ktsu-dev", "ProjectDirector"))); + + Assert.AreEqual(LocalPath(clone), chosen); + } + finally + { + TryDeleteDirectory(dev); + } + } + + [TestMethod] + public void AnOwnerScanUsesTheConventionalFolderForAnUnknownRepository() + { + FullyQualifiedLocalRepoPath conventional = LocalPath(Path.Combine(Path.GetTempPath(), "ktsu-dev", "New")); + + FullyQualifiedLocalRepoPath chosen = ProjectDirector.ChooseSyncedLocalPath(new Dictionary(), Name("New"), conventional); + + Assert.AreEqual(conventional, chosen); + } + + [TestMethod] + public void AnOwnerScanUsesTheConventionalFolderWhenTheKnownOneIsNotAClone() + { + string missing = Path.Combine(Path.GetTempPath(), $"ktsu_pd_{Guid.NewGuid():N}", "Gone"); + Dictionary repos = new() + { + [Name("Gone")] = Repository(missing, "Gone"), + }; + FullyQualifiedLocalRepoPath conventional = LocalPath(Path.Combine(Path.GetTempPath(), "ktsu-dev", "Gone")); + + FullyQualifiedLocalRepoPath chosen = ProjectDirector.ChooseSyncedLocalPath(repos, Name("Gone"), conventional); + + Assert.AreEqual(conventional, chosen); + } + + [TestMethod] + public void ACloneRecordedAtAPathItsRepositoryNoLongerUsesIsPruned() + { + string oldPath = Path.Combine(Path.GetTempPath(), "dev", "ProjectDirector"); + string newPath = Path.Combine(Path.GetTempPath(), "dev", "ktsu-dev", "ProjectDirector"); + Dictionary repos = new() + { + [Name("ProjectDirector")] = Repository(newPath, "ProjectDirector"), + }; + Dictionary cloned = new() + { + [LocalPath(oldPath)] = Name("ProjectDirector"), + }; + + Assert.IsTrue(ProjectDirector.PruneStaleClonedRepos(cloned, repos)); + Assert.IsEmpty(cloned); + } + + [TestMethod] + public void ACloneOfAnUnknownRepositoryIsPruned() + { + Dictionary cloned = new() + { + [LocalPath(Path.Combine(Path.GetTempPath(), "dev", "Orphan"))] = Name("Orphan"), + }; + + Assert.IsTrue(ProjectDirector.PruneStaleClonedRepos(cloned, new Dictionary())); + Assert.IsEmpty(cloned); + } + + [TestMethod] + public void ACloneRecordedAtItsRepositorysPathIsKept() + { + string path = Path.Combine(Path.GetTempPath(), "dev", "ProjectDirector"); + Dictionary repos = new() + { + [Name("ProjectDirector")] = Repository(path, "ProjectDirector"), + }; + Dictionary cloned = new() + { + [LocalPath(path)] = Name("ProjectDirector"), + }; + + Assert.IsFalse(ProjectDirector.PruneStaleClonedRepos(cloned, repos)); + Assert.HasCount(1, cloned); + } + + private static void TryDeleteDirectory(string path) + { + try + { + // Git marks objects read-only, which blocks a plain recursive delete on Windows. + foreach (string file in Directory.EnumerateFiles(path, "*", SearchOption.AllDirectories)) + { + File.SetAttributes(file, FileAttributes.Normal); + } + + Directory.Delete(path, recursive: true); + } + catch (IOException) + { + // A best-effort cleanup of a temp directory is not worth failing a test over. + } + catch (UnauthorizedAccessException) + { + // As above. + } + } +} diff --git a/ProjectDirector/ProjectDirector.cs b/ProjectDirector/ProjectDirector.cs index 6a244f0..f7cd061 100644 --- a/ProjectDirector/ProjectDirector.cs +++ b/ProjectDirector/ProjectDirector.cs @@ -1012,8 +1012,11 @@ private void SyncGitHubRepoInfoForOwner(GitHubOwnerName owner) foreach (Repository remoteRepo in remoteRepos) { - FullyQualifiedLocalRepoPath localPath = MakeFullyQualifyLocalRepoPath(Options.DevDirectory / RelativeDirectoryPath.Create(remoteRepo.FullName)); FullyQualifiedGitHubRepoName repoName = GetFullyQualifiedRepoName(remoteRepo); + FullyQualifiedLocalRepoPath localPath = ChooseSyncedLocalPath( + Options.Repos, + repoName, + MakeFullyQualifyLocalRepoPath(Options.DevDirectory / RelativeDirectoryPath.Create(remoteRepo.FullName))); GitRepository? repo = GitRepository.Create(GitRemotePath.Create(remoteRepo.CloneUrl), localPath); if (repo is not null) { @@ -1046,12 +1049,74 @@ private void UpdateClonedStatus() changed |= UpdateClonedStatus(repo); } + changed |= PruneStaleClonedRepos(Options.ClonedRepos, Options.Repos); + if (changed) { QueueSaveOptions(); } } + /// + /// Chooses where a repository listed by a GitHub owner scan lives on disk. + /// + /// The repositories already known. + /// The repository the scan listed. + /// Where the scan would put it: <dev>/<owner>/<repo>. + /// + /// The path the repository is already cloned at, if it is known and cloned, otherwise + /// . + /// + /// + /// Scan Dev Dir records a clone wherever it found it, often directly under the dev directory, and + /// adds the clone's owner to the owners it scans. An owner scan that then replaced the path with + /// the conventional one pointed the repository at a folder that does not exist, so every fetch of + /// it failed and its git actions acted on nothing. + /// + internal static FullyQualifiedLocalRepoPath ChooseSyncedLocalPath( + IReadOnlyDictionary repos, + FullyQualifiedGitHubRepoName repoName, + FullyQualifiedLocalRepoPath conventionalPath) + { + Ensure.NotNull(repos); + + return repos.TryGetValue(repoName, out GitRepository? existing) + && !string.IsNullOrEmpty(existing.LocalPath) + && GitCli.IsRepository(existing.LocalPath) + ? existing.LocalPath + : conventionalPath; + } + + /// + /// Removes the clones recorded at a path their repository no longer points at. + /// + /// The recorded clones, keyed by path. + /// The repositories they belong to. + /// if any entry was removed. + /// + /// Cloned status is otherwise updated only by walking at each + /// repository's current path, so an entry left at an old path is never visited and outlives the + /// change that moved its repository. + /// + internal static bool PruneStaleClonedRepos( + Dictionary clonedRepos, + IReadOnlyDictionary repos) + { + Ensure.NotNull(clonedRepos); + Ensure.NotNull(repos); + + List stale = [.. clonedRepos + .Where(entry => !repos.TryGetValue(entry.Value, out GitRepository? repo) || repo.LocalPath != entry.Key) + .Select(entry => entry.Key)]; + + foreach (FullyQualifiedLocalRepoPath path in stale) + { + _ = clonedRepos.Remove(path); + } + + return stale.Count > 0; + } + [System.Diagnostics.CodeAnalysis.SuppressMessage("Style", "IDE0045:Convert to conditional expression", Justification = "")] private bool UpdateClonedStatus(GitRepository repo) { From a1285daeda7d730187ac303c19b87b7674a6ba6e Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 17:35:34 +0000 Subject: [PATCH 2/2] Build the test paths with Path.Join Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01KLpJqJzDAe9WoXTVLt65CJ --- ProjectDirector.Test/OwnerSyncTests.cs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/ProjectDirector.Test/OwnerSyncTests.cs b/ProjectDirector.Test/OwnerSyncTests.cs index 4da3bca..436e6fb 100644 --- a/ProjectDirector.Test/OwnerSyncTests.cs +++ b/ProjectDirector.Test/OwnerSyncTests.cs @@ -34,7 +34,7 @@ private static FullyQualifiedLocalRepoPath LocalPath(string path) => }; private static string CreateDevDirectory() => - Directory.CreateDirectory(Path.Combine(Path.GetTempPath(), $"ktsu_pd_{Guid.NewGuid():N}")).FullName; + Directory.CreateDirectory(Path.Join(Path.GetTempPath(), $"ktsu_pd_{Guid.NewGuid():N}")).FullName; [TestMethod] public void AnOwnerScanKeepsTheFolderARepositoryIsAlreadyClonedIn() @@ -42,7 +42,7 @@ public void AnOwnerScanKeepsTheFolderARepositoryIsAlreadyClonedIn() string dev = CreateDevDirectory(); try { - string clone = Path.Combine(dev, "ProjectDirector"); + string clone = Path.Join(dev, "ProjectDirector"); Assert.IsTrue(GitCli.Run("init", clone).Succeeded, "git init failed."); Dictionary repos = new() { @@ -50,7 +50,7 @@ public void AnOwnerScanKeepsTheFolderARepositoryIsAlreadyClonedIn() }; FullyQualifiedLocalRepoPath chosen = ProjectDirector.ChooseSyncedLocalPath( - repos, Name("ProjectDirector"), LocalPath(Path.Combine(dev, "ktsu-dev", "ProjectDirector"))); + repos, Name("ProjectDirector"), LocalPath(Path.Join(dev, "ktsu-dev", "ProjectDirector"))); Assert.AreEqual(LocalPath(clone), chosen); } @@ -63,7 +63,7 @@ public void AnOwnerScanKeepsTheFolderARepositoryIsAlreadyClonedIn() [TestMethod] public void AnOwnerScanUsesTheConventionalFolderForAnUnknownRepository() { - FullyQualifiedLocalRepoPath conventional = LocalPath(Path.Combine(Path.GetTempPath(), "ktsu-dev", "New")); + FullyQualifiedLocalRepoPath conventional = LocalPath(Path.Join(Path.GetTempPath(), "ktsu-dev", "New")); FullyQualifiedLocalRepoPath chosen = ProjectDirector.ChooseSyncedLocalPath(new Dictionary(), Name("New"), conventional); @@ -73,12 +73,12 @@ public void AnOwnerScanUsesTheConventionalFolderForAnUnknownRepository() [TestMethod] public void AnOwnerScanUsesTheConventionalFolderWhenTheKnownOneIsNotAClone() { - string missing = Path.Combine(Path.GetTempPath(), $"ktsu_pd_{Guid.NewGuid():N}", "Gone"); + string missing = Path.Join(Path.GetTempPath(), $"ktsu_pd_{Guid.NewGuid():N}", "Gone"); Dictionary repos = new() { [Name("Gone")] = Repository(missing, "Gone"), }; - FullyQualifiedLocalRepoPath conventional = LocalPath(Path.Combine(Path.GetTempPath(), "ktsu-dev", "Gone")); + FullyQualifiedLocalRepoPath conventional = LocalPath(Path.Join(Path.GetTempPath(), "ktsu-dev", "Gone")); FullyQualifiedLocalRepoPath chosen = ProjectDirector.ChooseSyncedLocalPath(repos, Name("Gone"), conventional); @@ -88,8 +88,8 @@ public void AnOwnerScanUsesTheConventionalFolderWhenTheKnownOneIsNotAClone() [TestMethod] public void ACloneRecordedAtAPathItsRepositoryNoLongerUsesIsPruned() { - string oldPath = Path.Combine(Path.GetTempPath(), "dev", "ProjectDirector"); - string newPath = Path.Combine(Path.GetTempPath(), "dev", "ktsu-dev", "ProjectDirector"); + string oldPath = Path.Join(Path.GetTempPath(), "dev", "ProjectDirector"); + string newPath = Path.Join(Path.GetTempPath(), "dev", "ktsu-dev", "ProjectDirector"); Dictionary repos = new() { [Name("ProjectDirector")] = Repository(newPath, "ProjectDirector"), @@ -108,7 +108,7 @@ public void ACloneOfAnUnknownRepositoryIsPruned() { Dictionary cloned = new() { - [LocalPath(Path.Combine(Path.GetTempPath(), "dev", "Orphan"))] = Name("Orphan"), + [LocalPath(Path.Join(Path.GetTempPath(), "dev", "Orphan"))] = Name("Orphan"), }; Assert.IsTrue(ProjectDirector.PruneStaleClonedRepos(cloned, new Dictionary())); @@ -118,7 +118,7 @@ public void ACloneOfAnUnknownRepositoryIsPruned() [TestMethod] public void ACloneRecordedAtItsRepositorysPathIsKept() { - string path = Path.Combine(Path.GetTempPath(), "dev", "ProjectDirector"); + string path = Path.Join(Path.GetTempPath(), "dev", "ProjectDirector"); Dictionary repos = new() { [Name("ProjectDirector")] = Repository(path, "ProjectDirector"),