From 812eb596a6954d9822110da58b412a5caec79d5d Mon Sep 17 00:00:00 2001 From: Rui Marinho Date: Tue, 7 Apr 2026 17:36:01 +0100 Subject: [PATCH 1/3] fix: add existing repo clones locally and avoids duplicate sidebar groups Two bugs when adding a repo via 'Existing folder': 1. AddRepositoryFromLocalAsync was cloning from the remote URL over the network instead of the local path. Added localCloneSource parameter to AddRepositoryAsync that clones from the local path and then sets remote.origin.url to the real remote URL. 2. ReconcileOrganization was creating a duplicate URL-based repo group even when a local folder group already existed for the same repo. Added a check to prefer existing local folder groups before calling GetOrCreateRepoGroup. Fixes: MAUI.Sherpa added as existing repo still cloned and showed duplicate entries (folder + repo) in sidebar treeview. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- PolyPilot.Tests/AddExistingRepoTests.cs | 256 ++++++++++++++++++ .../Services/CopilotService.Organization.cs | 18 +- PolyPilot/Services/RepoManager.cs | 22 +- 3 files changed, 292 insertions(+), 4 deletions(-) create mode 100644 PolyPilot.Tests/AddExistingRepoTests.cs diff --git a/PolyPilot.Tests/AddExistingRepoTests.cs b/PolyPilot.Tests/AddExistingRepoTests.cs new file mode 100644 index 0000000000..2cfeffc0f9 --- /dev/null +++ b/PolyPilot.Tests/AddExistingRepoTests.cs @@ -0,0 +1,256 @@ +using PolyPilot.Models; +using PolyPilot.Services; +using Microsoft.Extensions.DependencyInjection; + +namespace PolyPilot.Tests; + +/// +/// Tests for the "Add Existing Repository" flow (AddRepositoryFromLocalAsync). +/// Covers two bugs: +/// 1. Adding an existing local repo should clone from the local path, not the remote URL. +/// 2. ReconcileOrganization should prefer a local folder group over creating a duplicate URL-based group. +/// +[Collection("BaseDir")] +public class AddExistingRepoTests +{ + private readonly StubChatDatabase _chatDb = new(); + private readonly StubServerManager _serverManager = new(); + private readonly StubWsBridgeClient _bridgeClient = new(); + private readonly StubDemoService _demoService = new(); + private readonly IServiceProvider _serviceProvider; + + public AddExistingRepoTests() + { + var services = new ServiceCollection(); + _serviceProvider = services.BuildServiceProvider(); + } + + private static RepoManager CreateRepoManagerWithState(List repos, List worktrees) + { + var rm = new RepoManager(); + var stateField = typeof(RepoManager).GetField("_state", + System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance)!; + var loadedField = typeof(RepoManager).GetField("_loaded", + System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance)!; + stateField.SetValue(rm, new RepositoryState { Repositories = repos, Worktrees = worktrees }); + loadedField.SetValue(rm, true); + return rm; + } + + private CopilotService CreateService(RepoManager? repoManager = null) => + new CopilotService(_chatDb, _serverManager, _bridgeClient, repoManager ?? new RepoManager(), _serviceProvider, _demoService); + + /// + /// Injects dummy SessionState entries into _sessions so ReconcileOrganization + /// doesn't hit the zero-session early-return guard. + /// + private static void AddDummySessions(CopilotService svc, params string[] names) + { + var sessionsField = typeof(CopilotService).GetField("_sessions", + System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance)!; + var dict = sessionsField.GetValue(svc)!; + var stateType = sessionsField.FieldType.GenericTypeArguments[1]; // SessionState + + foreach (var name in names) + { + var info = new AgentSessionInfo { Name = name, Model = "test-model" }; + var state = System.Runtime.CompilerServices.RuntimeHelpers.GetUninitializedObject(stateType); + stateType.GetProperty("Info")!.SetValue(state, info); + dict.GetType().GetMethod("TryAdd")!.Invoke(dict, new[] { name, state }); + } + } + + /// + /// Injects a SessionState with a specific working directory so ReconcileOrganization + /// can match it to a worktree via workingDir.StartsWith(w.Path). + /// + private static void AddDummySessionWithWorkingDir(CopilotService svc, string sessionName, string workingDirectory) + { + var sessionsField = typeof(CopilotService).GetField("_sessions", + System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance)!; + var dict = sessionsField.GetValue(svc)!; + var stateType = sessionsField.FieldType.GenericTypeArguments[1]; // SessionState + + var info = new AgentSessionInfo + { + Name = sessionName, + Model = "test-model", + WorkingDirectory = workingDirectory + }; + var state = System.Runtime.CompilerServices.RuntimeHelpers.GetUninitializedObject(stateType); + stateType.GetProperty("Info")!.SetValue(state, info); + dict.GetType().GetMethod("TryAdd")!.Invoke(dict, new[] { sessionName, (object)state }); + } + + // ─── Bug 2: ReconcileOrganization should prefer local folder groups ──────── + + [Fact] + public void Reconcile_SessionInDefault_WithLocalFolderGroupOnly_AssignsToLocalFolderGroup() + { + // Bug scenario: user added a repo via "Existing folder" (only a local folder group exists). + // A new session whose working dir matches the worktree should be assigned to the + // local folder group — NOT cause a new URL-based repo group to be created. + var localRepoPath = Path.Combine(Path.GetTempPath(), "MAUI.Sherpa"); + var nestedWtPath = Path.Combine(localRepoPath, ".polypilot", "worktrees", "session-1"); + + var repos = new List + { + new() { Id = "redth-MAUI.Sherpa", Name = "MAUI.Sherpa", Url = "https://github.com/redth/MAUI.Sherpa" } + }; + var worktrees = new List + { + new() { Id = "ext-1", RepoId = "redth-MAUI.Sherpa", Branch = "main", Path = localRepoPath }, + new() { Id = "wt-1", RepoId = "redth-MAUI.Sherpa", Branch = "feature", Path = nestedWtPath } + }; + var rm = CreateRepoManagerWithState(repos, worktrees); + var svc = CreateService(rm); + + // Only a local folder group exists (as when user added via "Existing folder") + var localGroup = svc.GetOrCreateLocalFolderGroup(localRepoPath, "redth-MAUI.Sherpa"); + Assert.True(localGroup.IsLocalFolder); + + // Session starts in default group, working in a nested worktree + var meta = new SessionMeta + { + SessionName = "MAUI.Sherpa", + GroupId = SessionGroup.DefaultId + }; + svc.Organization.Sessions.Add(meta); + AddDummySessionWithWorkingDir(svc, "MAUI.Sherpa", nestedWtPath); + + // Before reconcile: no URL-based group exists + var urlGroupsBefore = svc.Organization.Groups.Count(g => g.RepoId == "redth-MAUI.Sherpa" && !g.IsLocalFolder); + Assert.Equal(0, urlGroupsBefore); + + svc.ReconcileOrganization(); + + // After reconcile: session should be in the local folder group + var updatedMeta = svc.Organization.Sessions.First(m => m.SessionName == "MAUI.Sherpa"); + Assert.Equal(localGroup.Id, updatedMeta.GroupId); + + // No URL-based repo group should have been created + var urlGroupsAfter = svc.Organization.Groups.Count(g => g.RepoId == "redth-MAUI.Sherpa" && !g.IsLocalFolder && !g.IsMultiAgent); + Assert.Equal(0, urlGroupsAfter); + } + + [Fact] + public void Reconcile_SessionInDefault_WithBothGroupTypes_PrefersLocalFolderGroup() + { + // When both a local folder group and a URL-based group exist for the same repo, + // ReconcileOrganization should prefer the local folder group for unassigned sessions. + var localRepoPath = Path.Combine(Path.GetTempPath(), "MyProject"); + var nestedWtPath = Path.Combine(localRepoPath, ".polypilot", "worktrees", "feature-x"); + + var repos = new List + { + new() { Id = "repo-1", Name = "MyProject", Url = "https://github.com/test/myproject" } + }; + var worktrees = new List + { + new() { Id = "ext-1", RepoId = "repo-1", Branch = "main", Path = localRepoPath }, + new() { Id = "wt-1", RepoId = "repo-1", Branch = "feature-x", Path = nestedWtPath } + }; + var rm = CreateRepoManagerWithState(repos, worktrees); + var svc = CreateService(rm); + + // Both group types exist — local folder group takes priority + svc.GetOrCreateRepoGroup("repo-1", "MyProject"); + var localGroup = svc.GetOrCreateLocalFolderGroup(localRepoPath, "repo-1"); + + var meta = new SessionMeta + { + SessionName = "test-session", + GroupId = SessionGroup.DefaultId + }; + svc.Organization.Sessions.Add(meta); + AddDummySessionWithWorkingDir(svc, "test-session", nestedWtPath); + + svc.ReconcileOrganization(); + + var updated = svc.Organization.Sessions.First(m => m.SessionName == "test-session"); + Assert.Equal(localGroup.Id, updated.GroupId); + } + + [Fact] + public void Reconcile_SessionInDefault_WithOnlyUrlGroup_FallsBackToUrlGroup() + { + // When only a URL-based repo group exists (no local folder group), the session + // should be assigned to the URL-based group as before (existing behavior preserved). + var nestedWtPath = Path.Combine(Path.GetTempPath(), "worktrees", "feature-x"); + + var repos = new List + { + new() { Id = "repo-1", Name = "MyProject", Url = "https://github.com/test/myproject" } + }; + var worktrees = new List + { + new() { Id = "wt-1", RepoId = "repo-1", Branch = "feature-x", Path = nestedWtPath } + }; + var rm = CreateRepoManagerWithState(repos, worktrees); + var svc = CreateService(rm); + + // Only URL-based group exists + var urlGroup = svc.GetOrCreateRepoGroup("repo-1", "MyProject"); + + var meta = new SessionMeta + { + SessionName = "test-session", + GroupId = SessionGroup.DefaultId + }; + svc.Organization.Sessions.Add(meta); + AddDummySessionWithWorkingDir(svc, "test-session", nestedWtPath); + + svc.ReconcileOrganization(); + + var updated = svc.Organization.Sessions.First(m => m.SessionName == "test-session"); + Assert.Equal(urlGroup!.Id, updated.GroupId); + } + + // ─── Bug 1: AddRepositoryAsync supports local clone source ───────────────── + + [Fact] + public void AddRepositoryAsync_HasLocalCloneSourceOverload() + { + // Verify the internal overload with localCloneSource parameter exists and is accessible. + var method = typeof(RepoManager).GetMethod("AddRepositoryAsync", + System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance, + null, + new[] { typeof(string), typeof(Action), typeof(string), typeof(CancellationToken) }, + null); + Assert.NotNull(method); + } + + [Fact] + public void AddRepositoryFromLocalAsync_PassesLocalPathAsCloneSource() + { + // Verify that AddRepositoryFromLocalAsync calls AddRepositoryAsync with + // localCloneSource set to the local path (structural invariant). + // This ensures future refactors don't lose the local clone optimization. + var sourceFile = File.ReadAllText(Path.Combine(GetRepoRoot(), "PolyPilot", "Services", "RepoManager.cs")); + + // The call should pass localCloneSource: localPath + Assert.Contains("localCloneSource: localPath", sourceFile); + Assert.Contains("localCloneSource", sourceFile); + } + + [Fact] + public void AddRepositoryAsync_LocalCloneSource_SetsRemoteUrlAfterClone() + { + // Verify that when localCloneSource is used, the code sets the remote URL + // to the actual remote URL (not the local path) so future fetches go to the network. + var sourceFile = File.ReadAllText(Path.Combine(GetRepoRoot(), "PolyPilot", "Services", "RepoManager.cs")); + + // The local clone branch must reconfigure the remote origin + Assert.Contains("remote", sourceFile); + Assert.Contains("set-url", sourceFile); + Assert.Contains("origin", sourceFile); + } + + private static string GetRepoRoot() + { + var dir = AppContext.BaseDirectory; + while (dir != null && !File.Exists(Path.Combine(dir, "PolyPilot.slnx"))) + dir = Path.GetDirectoryName(dir); + return dir ?? throw new InvalidOperationException("Could not find repo root"); + } +} diff --git a/PolyPilot/Services/CopilotService.Organization.cs b/PolyPilot/Services/CopilotService.Organization.cs index b8c790b494..34fbdc4cfe 100644 --- a/PolyPilot/Services/CopilotService.Organization.cs +++ b/PolyPilot/Services/CopilotService.Organization.cs @@ -573,9 +573,21 @@ internal void ReconcileOrganization(bool allowPruning = true) var repo = _repoManager.Repositories.FirstOrDefault(r => r.Id == worktree.RepoId); if (repo != null) { - var repoGroup = GetOrCreateRepoGroup(repo.Id, repo.Name); - if (repoGroup != null) - meta.GroupId = repoGroup.Id; + // Prefer an existing local folder group for this repo over creating + // a new URL-based repo group. This prevents duplicate sidebar entries + // when the user added the repo via "Existing folder". + var localFolderGroup = Organization.Groups.FirstOrDefault(g => + g.RepoId == repo.Id && g.IsLocalFolder && !g.IsMultiAgent); + if (localFolderGroup != null) + { + meta.GroupId = localFolderGroup.Id; + } + else + { + var repoGroup = GetOrCreateRepoGroup(repo.Id, repo.Name); + if (repoGroup != null) + meta.GroupId = repoGroup.Id; + } } } changed = true; diff --git a/PolyPilot/Services/RepoManager.cs b/PolyPilot/Services/RepoManager.cs index 727415bed4..d9211a46e4 100644 --- a/PolyPilot/Services/RepoManager.cs +++ b/PolyPilot/Services/RepoManager.cs @@ -472,6 +472,14 @@ public Task AddRepositoryAsync(string url, CancellationToken ct => AddRepositoryAsync(url, null, ct); public async Task AddRepositoryAsync(string url, Action? onProgress, CancellationToken ct = default) + => await AddRepositoryAsync(url, onProgress, localCloneSource: null, ct); + + /// + /// When non-null, clone from this local path instead of the remote URL. + /// The remote origin is then set to so future fetches go to the network. + /// This avoids a redundant network clone when the user adds an existing local repository. + /// + internal async Task AddRepositoryAsync(string url, Action? onProgress, string? localCloneSource, CancellationToken ct = default) { url = NormalizeRepoUrl(url); EnsureLoaded(); @@ -494,6 +502,18 @@ public async Task AddRepositoryAsync(string url, Action? "+refs/heads/*:refs/remotes/origin/*"); } catch { } await RunGitWithProgressAsync(barePath, onProgress, ct, "fetch", "--progress", "origin"); } + else if (localCloneSource != null) + { + // Clone from local path — fast, no network required + onProgress?.Invoke($"Cloning from local folder…"); + await RunGitWithProgressAsync(null, onProgress, ct, "clone", "--bare", "--progress", localCloneSource, barePath); + + // Point remote origin at the real remote URL so future fetches go to the network + await RunGitAsync(barePath, ct, "remote", "set-url", "origin", url); + + await RunGitAsync(barePath, ct, "config", "remote.origin.fetch", + "+refs/heads/*:refs/remotes/origin/*"); + } else { onProgress?.Invoke($"Cloning {url}…"); @@ -577,7 +597,7 @@ public async Task AddRepositoryFromLocalAsync( $"No 'origin' remote found in '{localPath}'. " + "The folder must have a remote named 'origin' (e.g. a GitHub clone)."); - var repo = await AddRepositoryAsync(remoteUrl, onProgress, ct); + var repo = await AddRepositoryAsync(remoteUrl, onProgress, localCloneSource: localPath, ct); // Register the local folder as an external worktree so it also appears in the // "📂 Existing" picker when creating repo-based sessions. From 387a73e5bee16d7ae5e103ab82d461cbef3cc60b Mon Sep 17 00:00:00 2001 From: "Shane Neuville (HE/HIM)" Date: Tue, 7 Apr 2026 15:21:59 -0500 Subject: [PATCH 2/3] =?UTF-8?q?fix:=20address=20review=20findings=20?= =?UTF-8?q?=E2=80=94=20second=20reconcile=20block,=20tests,=20validation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Apply local-folder-group-first check to the second ReconcileOrganization block (WorktreeId-based re-link) to prevent duplicate sidebar groups via the group-deletion healing path - Replace source-file-reading tests with behavioral integration tests that create real git repos and verify local clone + remote URL wiring - Add test for the second reconcile block scenario - Add defensive validation for localCloneSource in internal overload - Add comment documenting intentional no-fetch design choice - Remove unused AddDummySessions helper - Fix unnecessary string interpolation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- PolyPilot.Tests/AddExistingRepoTests.cs | 194 +++++++++++++----- .../Services/CopilotService.Organization.cs | 17 +- PolyPilot/Services/RepoManager.cs | 9 +- 3 files changed, 166 insertions(+), 54 deletions(-) diff --git a/PolyPilot.Tests/AddExistingRepoTests.cs b/PolyPilot.Tests/AddExistingRepoTests.cs index 2cfeffc0f9..0844d592f1 100644 --- a/PolyPilot.Tests/AddExistingRepoTests.cs +++ b/PolyPilot.Tests/AddExistingRepoTests.cs @@ -40,26 +40,6 @@ private static RepoManager CreateRepoManagerWithState(List repos private CopilotService CreateService(RepoManager? repoManager = null) => new CopilotService(_chatDb, _serverManager, _bridgeClient, repoManager ?? new RepoManager(), _serviceProvider, _demoService); - /// - /// Injects dummy SessionState entries into _sessions so ReconcileOrganization - /// doesn't hit the zero-session early-return guard. - /// - private static void AddDummySessions(CopilotService svc, params string[] names) - { - var sessionsField = typeof(CopilotService).GetField("_sessions", - System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance)!; - var dict = sessionsField.GetValue(svc)!; - var stateType = sessionsField.FieldType.GenericTypeArguments[1]; // SessionState - - foreach (var name in names) - { - var info = new AgentSessionInfo { Name = name, Model = "test-model" }; - var state = System.Runtime.CompilerServices.RuntimeHelpers.GetUninitializedObject(stateType); - stateType.GetProperty("Info")!.SetValue(state, info); - dict.GetType().GetMethod("TryAdd")!.Invoke(dict, new[] { name, state }); - } - } - /// /// Injects a SessionState with a specific working directory so ReconcileOrganization /// can match it to a worktree via workingDir.StartsWith(w.Path). @@ -209,48 +189,166 @@ public void Reconcile_SessionInDefault_WithOnlyUrlGroup_FallsBackToUrlGroup() // ─── Bug 1: AddRepositoryAsync supports local clone source ───────────────── [Fact] - public void AddRepositoryAsync_HasLocalCloneSourceOverload() + public async Task AddRepositoryFromLocal_ClonesLocallyAndSetsRemoteUrl() + { + // Create a real local git repo with an origin remote, then call + // AddRepositoryFromLocalAsync and verify the bare clone's remote URL + // is the network URL (not the local path). + var tempDir = Path.Combine(Path.GetTempPath(), $"local-clone-test-{Guid.NewGuid():N}"); + var testBaseDir = Path.Combine(Path.GetTempPath(), $"rmtest-{Guid.NewGuid():N}"); + Directory.CreateDirectory(tempDir); + Directory.CreateDirectory(testBaseDir); + try + { + var remoteUrl = "https://github.com/test-owner/local-clone-test.git"; + + await RunProcess("git", "init", tempDir); + await RunProcess("git", "-C", tempDir, "config", "user.email", "test@test.com"); + await RunProcess("git", "-C", tempDir, "config", "user.name", "Test"); + await RunProcess("git", "-C", tempDir, "commit", "--allow-empty", "-m", "init"); + await RunProcess("git", "-C", tempDir, "remote", "add", "origin", remoteUrl); + + var rm = new RepoManager(); + RepoManager.SetBaseDirForTesting(testBaseDir); + try + { + var progressMessages = new List(); + var repo = await rm.AddRepositoryFromLocalAsync( + tempDir, msg => progressMessages.Add(msg)); + + // Should have used local clone, not network + Assert.Contains(progressMessages, m => m.Contains("local folder", StringComparison.OrdinalIgnoreCase)); + + // The bare clone's remote origin should point to the network URL + var bareRemoteUrl = await RunGitOutput(repo.BareClonePath, "remote", "get-url", "origin"); + Assert.Equal(remoteUrl, bareRemoteUrl.Trim()); + + // Verify the repo was registered + Assert.Contains(rm.Repositories, r => r.Id == repo.Id); + } + finally + { + RepoManager.SetBaseDirForTesting(TestSetup.TestBaseDir); + } + } + finally + { + ForceDeleteDirectory(tempDir); + ForceDeleteDirectory(testBaseDir); + } + } + + [Fact] + public void AddRepositoryAsync_LocalCloneSource_InvalidPath_Throws() { - // Verify the internal overload with localCloneSource parameter exists and is accessible. + var rm = new RepoManager(); var method = typeof(RepoManager).GetMethod("AddRepositoryAsync", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance, null, new[] { typeof(string), typeof(Action), typeof(string), typeof(CancellationToken) }, - null); - Assert.NotNull(method); + null)!; + + var ex = Assert.ThrowsAsync(async () => + await (Task)method.Invoke(rm, + new object?[] { "https://github.com/test/repo", null, "/nonexistent/path", CancellationToken.None })!); + + Assert.Contains("not found", ex.Result.Message, StringComparison.OrdinalIgnoreCase); } + // ─── Bug 2 (second block): WorktreeId-based reconcile prefers local folder ─ + [Fact] - public void AddRepositoryFromLocalAsync_PassesLocalPathAsCloneSource() + public void Reconcile_SessionWithWorktreeId_InDefault_WithLocalFolderGroup_AssignsToLocalGroup() + { + // When a session has a WorktreeId but is in the Default group (e.g., after + // group deletion healing), ReconcileOrganization should prefer the local + // folder group over creating a duplicate URL-based group. + var localRepoPath = Path.Combine(Path.GetTempPath(), "WorktreeIdTest"); + var nestedWtPath = Path.Combine(localRepoPath, ".polypilot", "worktrees", "wt-1"); + + var repos = new List + { + new() { Id = "test-wt-repo", Name = "WorktreeIdTest", Url = "https://github.com/test/worktreeidtest" } + }; + var worktrees = new List + { + new() { Id = "ext-1", RepoId = "test-wt-repo", Branch = "main", Path = localRepoPath }, + new() { Id = "wt-1", RepoId = "test-wt-repo", Branch = "feature", Path = nestedWtPath } + }; + var rm = CreateRepoManagerWithState(repos, worktrees); + var svc = CreateService(rm); + + // Create local folder group (as when user added via "Existing folder") + var localGroup = svc.GetOrCreateLocalFolderGroup(localRepoPath, "test-wt-repo"); + + // Session has a WorktreeId but is in Default (simulates group-deletion healing) + var meta = new SessionMeta + { + SessionName = "healed-session", + GroupId = SessionGroup.DefaultId, + WorktreeId = "wt-1" + }; + svc.Organization.Sessions.Add(meta); + AddDummySessionWithWorkingDir(svc, "healed-session", nestedWtPath); + + svc.ReconcileOrganization(); + + // Session should land in the local folder group, not a new URL-based group + var updated = svc.Organization.Sessions.First(m => m.SessionName == "healed-session"); + Assert.Equal(localGroup.Id, updated.GroupId); + + // No URL-based repo group should have been created + var urlGroups = svc.Organization.Groups.Count(g => + g.RepoId == "test-wt-repo" && !g.IsLocalFolder && !g.IsMultiAgent); + Assert.Equal(0, urlGroups); + } + + // ─── Helpers ─────────────────────────────────────────────────────────────── + + private static Task RunProcess(string exe, params string[] args) { - // Verify that AddRepositoryFromLocalAsync calls AddRepositoryAsync with - // localCloneSource set to the local path (structural invariant). - // This ensures future refactors don't lose the local clone optimization. - var sourceFile = File.ReadAllText(Path.Combine(GetRepoRoot(), "PolyPilot", "Services", "RepoManager.cs")); - - // The call should pass localCloneSource: localPath - Assert.Contains("localCloneSource: localPath", sourceFile); - Assert.Contains("localCloneSource", sourceFile); + var tcs = new TaskCompletionSource(); + var psi = new System.Diagnostics.ProcessStartInfo(exe) + { + RedirectStandardOutput = true, + RedirectStandardError = true, + UseShellExecute = false + }; + foreach (var a in args) psi.ArgumentList.Add(a); + var p = new System.Diagnostics.Process { StartInfo = psi, EnableRaisingEvents = true }; + p.Exited += (_, _) => + { + if (p.ExitCode == 0) tcs.TrySetResult(); + else tcs.TrySetException(new Exception($"{exe} exited with {p.ExitCode}")); + }; + p.Start(); + return tcs.Task; } - [Fact] - public void AddRepositoryAsync_LocalCloneSource_SetsRemoteUrlAfterClone() + private static async Task RunGitOutput(string workingDir, params string[] args) { - // Verify that when localCloneSource is used, the code sets the remote URL - // to the actual remote URL (not the local path) so future fetches go to the network. - var sourceFile = File.ReadAllText(Path.Combine(GetRepoRoot(), "PolyPilot", "Services", "RepoManager.cs")); - - // The local clone branch must reconfigure the remote origin - Assert.Contains("remote", sourceFile); - Assert.Contains("set-url", sourceFile); - Assert.Contains("origin", sourceFile); + var psi = new System.Diagnostics.ProcessStartInfo("git") + { + WorkingDirectory = workingDir, + RedirectStandardOutput = true, + RedirectStandardError = true, + UseShellExecute = false + }; + foreach (var a in args) psi.ArgumentList.Add(a); + var p = System.Diagnostics.Process.Start(psi)!; + var output = await p.StandardOutput.ReadToEndAsync(); + await p.WaitForExitAsync(); + if (p.ExitCode != 0) + throw new Exception($"git exited with {p.ExitCode}"); + return output; } - private static string GetRepoRoot() + private static void ForceDeleteDirectory(string path) { - var dir = AppContext.BaseDirectory; - while (dir != null && !File.Exists(Path.Combine(dir, "PolyPilot.slnx"))) - dir = Path.GetDirectoryName(dir); - return dir ?? throw new InvalidOperationException("Could not find repo root"); + if (!Directory.Exists(path)) + return; + foreach (var f in Directory.EnumerateFiles(path, "*", SearchOption.AllDirectories)) + File.SetAttributes(f, FileAttributes.Normal); + Directory.Delete(path, true); } } diff --git a/PolyPilot/Services/CopilotService.Organization.cs b/PolyPilot/Services/CopilotService.Organization.cs index 00169e52c5..d84679ce36 100644 --- a/PolyPilot/Services/CopilotService.Organization.cs +++ b/PolyPilot/Services/CopilotService.Organization.cs @@ -607,12 +607,21 @@ internal void ReconcileOrganization(bool allowPruning = true) var repo = _repoManager.Repositories.FirstOrDefault(r => r.Id == worktree.RepoId); if (repo != null) { - var repoGroup = GetOrCreateRepoGroup(repo.Id, repo.Name); - if (repoGroup != null) + // Prefer an existing local folder group (same fix as the + // workingDir-based block above) to avoid duplicate sidebar entries. + var localFolderGroup = Organization.Groups.FirstOrDefault(g => + g.RepoId == repo.Id && g.IsLocalFolder && !g.IsMultiAgent); + if (localFolderGroup != null) { - meta.GroupId = repoGroup.Id; - changed = true; + meta.GroupId = localFolderGroup.Id; } + else + { + var repoGroup = GetOrCreateRepoGroup(repo.Id, repo.Name); + if (repoGroup != null) + meta.GroupId = repoGroup.Id; + } + changed = true; } } } diff --git a/PolyPilot/Services/RepoManager.cs b/PolyPilot/Services/RepoManager.cs index d9211a46e4..a05e162413 100644 --- a/PolyPilot/Services/RepoManager.cs +++ b/PolyPilot/Services/RepoManager.cs @@ -481,6 +481,8 @@ public async Task AddRepositoryAsync(string url, Action? /// internal async Task AddRepositoryAsync(string url, Action? onProgress, string? localCloneSource, CancellationToken ct = default) { + if (localCloneSource != null && !Directory.Exists(localCloneSource)) + throw new ArgumentException($"Local clone source not found: '{localCloneSource}'", nameof(localCloneSource)); url = NormalizeRepoUrl(url); EnsureLoaded(); var id = RepoIdFromUrl(url); @@ -504,8 +506,11 @@ internal async Task AddRepositoryAsync(string url, Action Date: Tue, 7 Apr 2026 15:33:27 -0500 Subject: [PATCH 3/3] =?UTF-8?q?fix:=20address=20re-review=20=E2=80=94=20ch?= =?UTF-8?q?anged=3Dtrue=20regression,=20async=20test,=20Process=20leak?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Restore conditional changed=true in second reconcile block (only set when a group assignment actually happens, matching original behavior) - Make AddRepositoryAsync_LocalCloneSource_InvalidPath_Throws async instead of using .Result on the ThrowsAsync task - Add using to Process in RunGitOutput to prevent handle leaks Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- PolyPilot.Tests/AddExistingRepoTests.cs | 8 ++++---- PolyPilot/Services/CopilotService.Organization.cs | 5 ++++- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/PolyPilot.Tests/AddExistingRepoTests.cs b/PolyPilot.Tests/AddExistingRepoTests.cs index 0844d592f1..825a23f7a5 100644 --- a/PolyPilot.Tests/AddExistingRepoTests.cs +++ b/PolyPilot.Tests/AddExistingRepoTests.cs @@ -239,7 +239,7 @@ public async Task AddRepositoryFromLocal_ClonesLocallyAndSetsRemoteUrl() } [Fact] - public void AddRepositoryAsync_LocalCloneSource_InvalidPath_Throws() + public async Task AddRepositoryAsync_LocalCloneSource_InvalidPath_Throws() { var rm = new RepoManager(); var method = typeof(RepoManager).GetMethod("AddRepositoryAsync", @@ -248,11 +248,11 @@ public void AddRepositoryAsync_LocalCloneSource_InvalidPath_Throws() new[] { typeof(string), typeof(Action), typeof(string), typeof(CancellationToken) }, null)!; - var ex = Assert.ThrowsAsync(async () => + var ex = await Assert.ThrowsAsync(async () => await (Task)method.Invoke(rm, new object?[] { "https://github.com/test/repo", null, "/nonexistent/path", CancellationToken.None })!); - Assert.Contains("not found", ex.Result.Message, StringComparison.OrdinalIgnoreCase); + Assert.Contains("not found", ex.Message, StringComparison.OrdinalIgnoreCase); } // ─── Bug 2 (second block): WorktreeId-based reconcile prefers local folder ─ @@ -335,7 +335,7 @@ private static async Task RunGitOutput(string workingDir, params string[ UseShellExecute = false }; foreach (var a in args) psi.ArgumentList.Add(a); - var p = System.Diagnostics.Process.Start(psi)!; + using var p = System.Diagnostics.Process.Start(psi)!; var output = await p.StandardOutput.ReadToEndAsync(); await p.WaitForExitAsync(); if (p.ExitCode != 0) diff --git a/PolyPilot/Services/CopilotService.Organization.cs b/PolyPilot/Services/CopilotService.Organization.cs index d84679ce36..b5cb588bb2 100644 --- a/PolyPilot/Services/CopilotService.Organization.cs +++ b/PolyPilot/Services/CopilotService.Organization.cs @@ -614,14 +614,17 @@ internal void ReconcileOrganization(bool allowPruning = true) if (localFolderGroup != null) { meta.GroupId = localFolderGroup.Id; + changed = true; } else { var repoGroup = GetOrCreateRepoGroup(repo.Id, repo.Name); if (repoGroup != null) + { meta.GroupId = repoGroup.Id; + changed = true; + } } - changed = true; } } }