From 62a9c086cee525a36a5638c302f4978374dfd8a9 Mon Sep 17 00:00:00 2001 From: Shane Neuville Date: Sat, 18 Apr 2026 08:17:30 -0500 Subject: [PATCH 1/3] fix: prevent duplicate URL group for local-only repos MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GetOrCreateRepoGroup created a duplicate URL-based group for repos that only exist as local folder repos (e.g., dotnet-maui-local-*). This caused two 'maui' groups to appear in the sidebar — the local folder group and a newly created URL-based group with the same name. The fix: when a local folder group already covers a repo with a '-local-' ID suffix, GetOrCreateRepoGroup returns null instead of creating a redundant URL-based group. Repos with managed bare clones (same repoId for both local and URL groups) still allow URL group creation for the heal-stranded-sessions scenario. Adds regression test verifying that local-only repos don't get duplicate URL-based groups during ReconcileOrganization. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- PolyPilot.Tests/SessionOrganizationTests.cs | 41 +++++++++++++++++++ .../Services/CopilotService.Organization.cs | 14 +++++++ 2 files changed, 55 insertions(+) diff --git a/PolyPilot.Tests/SessionOrganizationTests.cs b/PolyPilot.Tests/SessionOrganizationTests.cs index 2f1586f45..144ccef40 100644 --- a/PolyPilot.Tests/SessionOrganizationTests.cs +++ b/PolyPilot.Tests/SessionOrganizationTests.cs @@ -1098,6 +1098,47 @@ public void ReconcileOrganization_ExternalWorktree_DoesNotPromoteWhenLocalGroupA localGroup.LocalPath); } + [Fact] + public void ReconcileOrganization_LocalOnlyRepo_DoesNotCreateUrlGroup_ForCentralizedWorktree() + { + // Regression test: when a repo ONLY has a local folder group (no URL-based group), + // sessions with centralized worktrees (~/.polypilot/worktrees/...) should stay in + // the local folder group. ReconcileOrganization must NOT create a duplicate URL-based + // group just to move the session into it. + var repos = new List + { + new() { Id = "local-repo-abc123", Name = "maui", Url = "https://github.com/dotnet/maui" } + }; + var localPath = Path.Combine(Path.GetTempPath(), "maui3"); + var centralPath = Path.Combine(Path.GetTempPath(), ".polypilot", "worktrees", "local-repo-wt1"); + var worktrees = new List + { + new() { Id = "wt-central", RepoId = "local-repo-abc123", Branch = "session-123", Path = centralPath } + }; + var rm = CreateRepoManagerWithState(repos, worktrees); + var svc = CreateService(rm); + + // Create ONLY a local folder group (no URL-based group) + var localGroup = svc.GetOrCreateLocalFolderGroup(localPath, "local-repo-abc123"); + Assert.True(localGroup.IsLocalFolder); + + // Put a session in the local folder group with a centralized worktree + var meta = new SessionMeta + { + SessionName = "test-session", + GroupId = localGroup.Id, + WorktreeId = "wt-central" + }; + svc.Organization.Sessions.Add(meta); + + svc.ReconcileOrganization(); + + // Session should stay in the local folder group — no URL group created + Assert.Equal(localGroup.Id, meta.GroupId); + Assert.DoesNotContain(svc.Organization.Groups, + g => g.RepoId == "local-repo-abc123" && !g.IsLocalFolder && !g.IsMultiAgent); + } + [Fact] public void ReconcileOrganization_NestedWorktree_IsNotTreatedAsExternalWorktree() { diff --git a/PolyPilot/Services/CopilotService.Organization.cs b/PolyPilot/Services/CopilotService.Organization.cs index 570b0d0d9..81c075010 100644 --- a/PolyPilot/Services/CopilotService.Organization.cs +++ b/PolyPilot/Services/CopilotService.Organization.cs @@ -1438,6 +1438,20 @@ internal bool IsWorkerInMultiAgentGroup(string sessionName) if (!explicitly && Organization.DeletedRepoGroupRepoIds.Contains(repoId)) return null; + // Don't create a URL-based group when a local folder group already covers this repo + // and no URL-based group exists. This prevents duplicate sidebar entries for local-only + // repos (e.g., "maui" appearing twice — once as local folder, once as URL-based). + if (!explicitly && Organization.Groups.Any(g => g.RepoId == repoId && g.IsLocalFolder && !g.IsMultiAgent)) + { + // Exception: if the local folder group is for the SAME repo (same ID, not a separate + // local-* ID), allow creating a URL group. This supports the heal-stranded-sessions + // scenario where sessions with managed worktrees need a URL group to live in. + var localRepo = _repoManager?.Repositories.FirstOrDefault(r => r.Id == repoId); + var isLocalOnlyRepo = localRepo != null && repoId.Contains("-local-", StringComparison.Ordinal); + if (isLocalOnlyRepo) + return null; + } + // Clear the deleted flag when explicitly re-adding Organization.DeletedRepoGroupRepoIds.Remove(repoId); From 46f298802d1baa7bc2539fb2a32838a890538f43 Mon Sep 17 00:00:00 2001 From: Shane Neuville Date: Sat, 18 Apr 2026 13:32:45 -0500 Subject: [PATCH 2/3] =?UTF-8?q?fix:=20address=20review=20findings=20?= =?UTF-8?q?=E2=80=94=20correct=20test=20IDs,=20simplify=20guard?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix test repo ID to use real '-local-' format ('dotnet-maui-local-a1b2c3d4') so the guard code is actually exercised (was 'local-repo-abc123' which doesn't contain '-local-' substring) - Set IsInitialized + allowPruning:false in integration test so ReconcileOrganization doesn't early-exit before reaching heal loop - Remove unnecessary localRepo != null check — repoId.Contains('-local-') alone is sufficient, avoids silent failure when repo missing from state - Simplify guard: check '-local-' first, then verify local folder group exists (clearer intent, fewer LINQ scans on non-local repos) - Add direct GetOrCreateRepoGroup unit test for local-only repos (null) - Add direct GetOrCreateRepoGroup unit test for non-local repos (allowed) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- PolyPilot.Tests/SessionOrganizationTests.cs | 51 ++++++++++++++++--- .../Services/CopilotService.Organization.cs | 18 +++---- 2 files changed, 54 insertions(+), 15 deletions(-) diff --git a/PolyPilot.Tests/SessionOrganizationTests.cs b/PolyPilot.Tests/SessionOrganizationTests.cs index 144ccef40..4725e08a6 100644 --- a/PolyPilot.Tests/SessionOrganizationTests.cs +++ b/PolyPilot.Tests/SessionOrganizationTests.cs @@ -1105,21 +1105,22 @@ public void ReconcileOrganization_LocalOnlyRepo_DoesNotCreateUrlGroup_ForCentral // sessions with centralized worktrees (~/.polypilot/worktrees/...) should stay in // the local folder group. ReconcileOrganization must NOT create a duplicate URL-based // group just to move the session into it. + // Note: repo ID must use the real "-local-" format from RepoManager.AddRepositoryFromLocalAsync. var repos = new List { - new() { Id = "local-repo-abc123", Name = "maui", Url = "https://github.com/dotnet/maui" } + new() { Id = "dotnet-maui-local-a1b2c3d4", Name = "maui", Url = "https://github.com/dotnet/maui" } }; var localPath = Path.Combine(Path.GetTempPath(), "maui3"); - var centralPath = Path.Combine(Path.GetTempPath(), ".polypilot", "worktrees", "local-repo-wt1"); + var centralPath = Path.Combine(Path.GetTempPath(), ".polypilot", "worktrees", "dotnet-maui-local-wt1"); var worktrees = new List { - new() { Id = "wt-central", RepoId = "local-repo-abc123", Branch = "session-123", Path = centralPath } + new() { Id = "wt-central", RepoId = "dotnet-maui-local-a1b2c3d4", Branch = "session-123", Path = centralPath } }; var rm = CreateRepoManagerWithState(repos, worktrees); var svc = CreateService(rm); // Create ONLY a local folder group (no URL-based group) - var localGroup = svc.GetOrCreateLocalFolderGroup(localPath, "local-repo-abc123"); + var localGroup = svc.GetOrCreateLocalFolderGroup(localPath, "dotnet-maui-local-a1b2c3d4"); Assert.True(localGroup.IsLocalFolder); // Put a session in the local folder group with a centralized worktree @@ -1131,12 +1132,50 @@ public void ReconcileOrganization_LocalOnlyRepo_DoesNotCreateUrlGroup_ForCentral }; svc.Organization.Sessions.Add(meta); - svc.ReconcileOrganization(); + typeof(CopilotService).GetProperty("IsInitialized")!.SetValue(svc, true); + svc.ReconcileOrganization(allowPruning: false); // Session should stay in the local folder group — no URL group created Assert.Equal(localGroup.Id, meta.GroupId); Assert.DoesNotContain(svc.Organization.Groups, - g => g.RepoId == "local-repo-abc123" && !g.IsLocalFolder && !g.IsMultiAgent); + g => g.RepoId == "dotnet-maui-local-a1b2c3d4" && !g.IsLocalFolder && !g.IsMultiAgent); + } + + [Fact] + public void GetOrCreateRepoGroup_ReturnsNull_ForLocalOnlyRepoWithExistingLocalGroup() + { + // Direct unit test: GetOrCreateRepoGroup must return null for local-only repos + // (IDs containing "-local-") when a local folder group already covers the repo. + var svc = CreateService(); + var localPath = Path.Combine(Path.GetTempPath(), "my-project"); + + // Create a local folder group for a local-only repo + svc.GetOrCreateLocalFolderGroup(localPath, "owner-repo-local-a1b2c3d4"); + + // GetOrCreateRepoGroup should return null — not create a duplicate + var result = svc.GetOrCreateRepoGroup("owner-repo-local-a1b2c3d4", "repo"); + Assert.Null(result); + + // No URL-based group should exist + Assert.DoesNotContain(svc.Organization.Groups, + g => g.RepoId == "owner-repo-local-a1b2c3d4" && !g.IsLocalFolder); + } + + [Fact] + public void GetOrCreateRepoGroup_AllowsCreation_ForNonLocalRepo_WithLocalFolderGroup() + { + // Ensure the guard does NOT block URL group creation for repos without "-local-" in ID. + // This supports the heal-stranded-sessions scenario where both URL and local groups coexist. + var svc = CreateService(); + var localPath = Path.Combine(Path.GetTempPath(), "my-project"); + + // Create a local folder group for a non-local repo (same ID for both) + svc.GetOrCreateLocalFolderGroup(localPath, "owner-repo"); + + // GetOrCreateRepoGroup should succeed — this is NOT a local-only repo + var result = svc.GetOrCreateRepoGroup("owner-repo", "repo"); + Assert.NotNull(result); + Assert.False(result!.IsLocalFolder); } [Fact] diff --git a/PolyPilot/Services/CopilotService.Organization.cs b/PolyPilot/Services/CopilotService.Organization.cs index 81c075010..c078c2484 100644 --- a/PolyPilot/Services/CopilotService.Organization.cs +++ b/PolyPilot/Services/CopilotService.Organization.cs @@ -1441,15 +1441,15 @@ internal bool IsWorkerInMultiAgentGroup(string sessionName) // Don't create a URL-based group when a local folder group already covers this repo // and no URL-based group exists. This prevents duplicate sidebar entries for local-only // repos (e.g., "maui" appearing twice — once as local folder, once as URL-based). - if (!explicitly && Organization.Groups.Any(g => g.RepoId == repoId && g.IsLocalFolder && !g.IsMultiAgent)) - { - // Exception: if the local folder group is for the SAME repo (same ID, not a separate - // local-* ID), allow creating a URL group. This supports the heal-stranded-sessions - // scenario where sessions with managed worktrees need a URL group to live in. - var localRepo = _repoManager?.Repositories.FirstOrDefault(r => r.Id == repoId); - var isLocalOnlyRepo = localRepo != null && repoId.Contains("-local-", StringComparison.Ordinal); - if (isLocalOnlyRepo) - return null; + // Local-only repos have IDs like "dotnet-maui-local-a1b2c3d4" (the "-local-" infix is + // added by RepoManager.AddRepositoryFromLocalAsync). For these repos, the local folder + // group IS the repo's group — no URL-based group should be created. + // Exception: repos WITHOUT "-local-" in their ID (same ID for both URL and local groups) + // are allowed to create URL groups for the heal-stranded-sessions scenario. + if (!explicitly && repoId.Contains("-local-", StringComparison.Ordinal) + && Organization.Groups.Any(g => g.RepoId == repoId && g.IsLocalFolder && !g.IsMultiAgent)) + { + return null; } // Clear the deleted flag when explicitly re-adding From d97f0c6952eda15ba53c26fd3182b49d84a23ce3 Mon Sep 17 00:00:00 2001 From: Shane Neuville Date: Sat, 18 Apr 2026 14:22:15 -0500 Subject: [PATCH 3/3] fix: replace magic string with shared constant and add diagnostic log - Add RepoManager.LocalRepoIdInfix constant ("-local-") and IsLocalOnlyRepoId() helper method - Replace all 5 inline "-local-" usages across RepoManager.cs and CopilotService.Organization.cs with the constant/helper - Add diagnostic Debug log when heal-stranded-sessions is intentionally blocked for local-only repos (aids troubleshooting) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Services/CopilotService.Organization.cs | 12 ++++++++---- PolyPilot/Services/RepoManager.cs | 18 ++++++++++++++---- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/PolyPilot/Services/CopilotService.Organization.cs b/PolyPilot/Services/CopilotService.Organization.cs index c078c2484..fb45599fa 100644 --- a/PolyPilot/Services/CopilotService.Organization.cs +++ b/PolyPilot/Services/CopilotService.Organization.cs @@ -808,6 +808,10 @@ internal void ReconcileOrganization(bool allowPruning = true) meta.GroupId = urlGroup.Id; changed = true; } + else + { + Debug($"ReconcileOrganization: keeping '{meta.SessionName}' in local folder group '{localGroup.Name}' — no URL group for local-only repo '{localGroup.RepoId}'"); + } } } } @@ -1441,12 +1445,12 @@ internal bool IsWorkerInMultiAgentGroup(string sessionName) // Don't create a URL-based group when a local folder group already covers this repo // and no URL-based group exists. This prevents duplicate sidebar entries for local-only // repos (e.g., "maui" appearing twice — once as local folder, once as URL-based). - // Local-only repos have IDs like "dotnet-maui-local-a1b2c3d4" (the "-local-" infix is - // added by RepoManager.AddRepositoryFromLocalAsync). For these repos, the local folder + // Local-only repos are identified by RepoManager.IsLocalOnlyRepoId (IDs containing the + // "-local-" infix from AddRepositoryFromLocalAsync). For these repos, the local folder // group IS the repo's group — no URL-based group should be created. - // Exception: repos WITHOUT "-local-" in their ID (same ID for both URL and local groups) + // Exception: repos WITHOUT the local infix (same ID for both URL and local groups) // are allowed to create URL groups for the heal-stranded-sessions scenario. - if (!explicitly && repoId.Contains("-local-", StringComparison.Ordinal) + if (!explicitly && RepoManager.IsLocalOnlyRepoId(repoId) && Organization.Groups.Any(g => g.RepoId == repoId && g.IsLocalFolder && !g.IsMultiAgent)) { return null; diff --git a/PolyPilot/Services/RepoManager.cs b/PolyPilot/Services/RepoManager.cs index 82ea75679..8d9e3b853 100644 --- a/PolyPilot/Services/RepoManager.cs +++ b/PolyPilot/Services/RepoManager.cs @@ -14,6 +14,16 @@ namespace PolyPilot.Services; /// public class RepoManager { + /// + /// Infix used in repo IDs for local folder repos (e.g., "dotnet-maui-local-a1b2c3d4"). + /// Generated by when a URL-based repo already exists. + /// + internal const string LocalRepoIdInfix = "-local-"; + + /// Returns true if the repo ID identifies a local-only repo (added via "Existing Folder"). + internal static bool IsLocalOnlyRepoId(string repoId) => + repoId.Contains(LocalRepoIdInfix, StringComparison.Ordinal); + private static string? _baseDirOverride; private static readonly object _pathLock = new(); private static string? _stateFile; @@ -456,7 +466,7 @@ public static string RepoNameFromUrl(string? url, string? fallbackId = null) // Strip "-local-{hash}" suffix before deriving name so local repo IDs like // "dotnet-maui-local-a1b2c3d4" produce "maui" instead of "maui-local-a1b2c3d4". var cleanId = fallbackId; - var localIdx = cleanId.IndexOf("-local-", StringComparison.Ordinal); + var localIdx = cleanId.IndexOf(LocalRepoIdInfix, StringComparison.Ordinal); if (localIdx > 0) cleanId = cleanId[..localIdx]; var dashIdx = cleanId.IndexOf('-'); @@ -501,7 +511,7 @@ internal static string WorktreeDirName(string repoId, string worktreeId) // Strip "-local-{hash}" suffixes from "Existing Folder" repo IDs to shorten the path. // e.g., "dotnet-maui-local-a1b2c3d4" → "dotnet-maui" var abbreviated = repoId; - var localIdx = abbreviated.IndexOf("-local-", StringComparison.Ordinal); + var localIdx = abbreviated.IndexOf(LocalRepoIdInfix, StringComparison.Ordinal); if (localIdx > 0) abbreviated = abbreviated[..localIdx]; @@ -802,7 +812,7 @@ public async Task AddRepositoryFromLocalAsync( // Use SHA256 for a deterministic hash — string.GetHashCode() is randomized per-process // in .NET Core 3.0+ and must not be persisted. var pathHash = DeterministicPathHash(localPath); - var localId = $"{baseId}-local-{pathHash}"; + var localId = $"{baseId}{LocalRepoIdInfix}{pathHash}"; // Ensure we don't collide with an existing entry with this generated ID. // Idempotency is guaranteed by the `existingLocal` path-match check above (line 781), @@ -816,7 +826,7 @@ public async Task AddRepositoryFromLocalAsync( else if (alreadyLocal != null) { // True hash collision — same ID, different path. Disambiguate with GUID. - localId = $"{baseId}-local-{Guid.NewGuid().ToString("N")[..8]}"; + localId = $"{baseId}{LocalRepoIdInfix}{Guid.NewGuid().ToString("N")[..8]}"; repo = new RepositoryInfo { Id = localId,