From 64ef364e18baea08bd567f52fa25208a76e95fd7 Mon Sep 17 00:00:00 2001 From: Shane Neuville Date: Fri, 17 Apr 2026 22:26:38 -0500 Subject: [PATCH 1/2] fix: preserve group name when promoting to local folder group MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PromoteOrCreateLocalFolderGroup and ReconcileOrganization's external worktree migration both unconditionally renamed groups to Path.GetFileName(worktreePath) during promotion. This destroyed user-customized group names — e.g., a group named 'maui' was renamed to 'maui2' because the local folder was ~/Projects/maui2. Remove the Name override in both locations. Promotion should only set LocalPath to convert the group to a local folder group, not change the user's chosen name. Adds two regression tests verifying name preservation in both the explicit promote path and the ReconcileOrganization migration path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- PolyPilot.Tests/SessionOrganizationTests.cs | 55 +++++++++++++++++++ .../Services/CopilotService.Organization.cs | 12 ++-- 2 files changed, 63 insertions(+), 4 deletions(-) diff --git a/PolyPilot.Tests/SessionOrganizationTests.cs b/PolyPilot.Tests/SessionOrganizationTests.cs index b479d3c5f..8e65f2a9e 100644 --- a/PolyPilot.Tests/SessionOrganizationTests.cs +++ b/PolyPilot.Tests/SessionOrganizationTests.cs @@ -933,6 +933,61 @@ public void PromoteOrCreateLocalFolderGroup_PromotesMostRecentUrlGroup_WhenMulti Assert.False(olderGroup!.IsLocalFolder); // older group stays URL-based } + [Fact] + public void PromoteOrCreateLocalFolderGroup_PreservesUserGroupName_WhenFolderNameDiffers() + { + // Regression test: when the user's group is named "maui" and the local folder + // is "~/Projects/maui2", promotion must NOT rename the group to "maui2". + var svc = CreateService(); + var localRepoPath = Path.Combine(Path.GetTempPath(), "maui2"); + var expectedPath = Path.GetFullPath(localRepoPath) + .TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); + + // Simulate: user has a group named "maui" for repo "dotnet-maui" + var urlGroup = svc.GetOrCreateRepoGroup("dotnet-maui", "maui"); + Assert.NotNull(urlGroup); + Assert.Equal("maui", urlGroup!.Name); + + // Promote with a folder whose basename is "maui2" — NOT "maui" + var result = svc.PromoteOrCreateLocalFolderGroup(localRepoPath, "dotnet-maui"); + + Assert.Equal(urlGroup.Id, result.Id); + Assert.True(result.IsLocalFolder); + Assert.Equal(expectedPath, result.LocalPath); + // Name must be preserved — NOT overwritten with "maui2" + Assert.Equal("maui", result.Name); + } + + [Fact] + public void ReconcileOrganization_ExternalWorktree_PreservesGroupName_WhenPromoting() + { + // Regression test: ReconcileOrganization's external worktree migration must + // preserve the group's name when promoting it to a local folder group. + var repos = new List + { + new() { Id = "dotnet-maui", Name = "maui", Url = "https://github.com/dotnet/maui" } + }; + // External worktree folder name is "maui2" — differs from group name "maui" + var extPath = Path.Combine(Path.GetTempPath(), "maui2"); + var worktrees = new List + { + new() { Id = "ext-1", RepoId = "dotnet-maui", Branch = "main", Path = extPath } + }; + var rm = CreateRepoManagerWithState(repos, worktrees); + var svc = CreateService(rm); + + // Create a URL-based group named "maui" (user's custom name) + var urlGroup = svc.GetOrCreateRepoGroup("dotnet-maui", "maui"); + Assert.Equal("maui", urlGroup!.Name); + + svc.ReconcileOrganization(); + + var promoted = svc.Organization.Groups.First(g => g.Id == urlGroup.Id); + Assert.True(promoted.IsLocalFolder); + // Name must be "maui" — NOT "maui2" (the folder basename) + Assert.Equal("maui", promoted.Name); + } + [Fact] public void ReconcileOrganization_ExternalWorktree_PromotesUrlGroupToLocalFolderGroup() { diff --git a/PolyPilot/Services/CopilotService.Organization.cs b/PolyPilot/Services/CopilotService.Organization.cs index 9c8fe1ef2..b6a7a9671 100644 --- a/PolyPilot/Services/CopilotService.Organization.cs +++ b/PolyPilot/Services/CopilotService.Organization.cs @@ -741,9 +741,11 @@ internal void ReconcileOrganization(bool allowPruning = true) if (groupToPromote != null) { groupToPromote.LocalPath = normalizedExtPath; - groupToPromote.Name = Path.GetFileName(normalizedExtPath); + // Preserve the user's group name — don't overwrite with the folder basename. + // The old code did: groupToPromote.Name = Path.GetFileName(normalizedExtPath) + // which destroyed user-customized names (e.g., "maui" → "maui2"). changed = true; - Debug($"ReconcileOrganization: promoted group '{groupToPromote.Id}' to local folder group for '{normalizedExtPath}'"); + Debug($"ReconcileOrganization: promoted group '{groupToPromote.Id}' ('{groupToPromote.Name}') to local folder group for '{normalizedExtPath}'"); // Migrate sessions whose worktrees are NOT under the new LocalPath to the // URL-based repo group. Without this, sessions linked to managed worktrees @@ -1533,10 +1535,12 @@ public SessionGroup PromoteOrCreateLocalFolderGroup(string localPath, string? re if (candidate != null) { candidate.LocalPath = normalized; - candidate.Name = Path.GetFileName(normalized); + // Preserve the user's group name — don't overwrite with the folder basename. + // The old code did: candidate.Name = Path.GetFileName(normalized) + // which destroyed user-customized names (e.g., "maui" → "maui2"). SaveOrganization(); OnStateChanged?.Invoke(); - Debug($"PromoteOrCreateLocalFolderGroup: promoted '{candidate.Id}' to local folder group for '{normalized}'"); + Debug($"PromoteOrCreateLocalFolderGroup: promoted '{candidate.Id}' ('{candidate.Name}') to local folder group for '{normalized}'"); return candidate; } } From c9bcc64e71950439817cf51e415f73570cea7739 Mon Sep 17 00:00:00 2001 From: Shane Neuville Date: Fri, 17 Apr 2026 23:46:48 -0500 Subject: [PATCH 2/2] =?UTF-8?q?fix:=20address=20review=20findings=20?= =?UTF-8?q?=E2=80=94=20empty-name=20fallback,=20asymmetry=20docs,=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add defensive fallback: if group has empty/whitespace name during promotion, fall back to Path.GetFileName (both code paths) - Document intentional naming asymmetry between promotion (preserves existing name) and creation (uses folder basename) with inline comment - Add test for creation path verifying folder basename is used - Add test for empty-name fallback verifying repair behavior - Add 'path need not exist on disk' notes to test comments Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- PolyPilot.Tests/SessionOrganizationTests.cs | 44 +++++++++++++++++++ .../Services/CopilotService.Organization.cs | 10 +++++ 2 files changed, 54 insertions(+) diff --git a/PolyPilot.Tests/SessionOrganizationTests.cs b/PolyPilot.Tests/SessionOrganizationTests.cs index 8e65f2a9e..2f1586f45 100644 --- a/PolyPilot.Tests/SessionOrganizationTests.cs +++ b/PolyPilot.Tests/SessionOrganizationTests.cs @@ -939,6 +939,7 @@ public void PromoteOrCreateLocalFolderGroup_PreservesUserGroupName_WhenFolderNam // Regression test: when the user's group is named "maui" and the local folder // is "~/Projects/maui2", promotion must NOT rename the group to "maui2". var svc = CreateService(); + // Note: path need not exist on disk; promotion matches on repoId, not disk state. var localRepoPath = Path.Combine(Path.GetTempPath(), "maui2"); var expectedPath = Path.GetFullPath(localRepoPath) .TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); @@ -968,6 +969,7 @@ public void ReconcileOrganization_ExternalWorktree_PreservesGroupName_WhenPromot new() { Id = "dotnet-maui", Name = "maui", Url = "https://github.com/dotnet/maui" } }; // External worktree folder name is "maui2" — differs from group name "maui" + // Note: path need not exist on disk; ReconcileOrganization matches on repoId, not disk state. var extPath = Path.Combine(Path.GetTempPath(), "maui2"); var worktrees = new List { @@ -988,6 +990,48 @@ public void ReconcileOrganization_ExternalWorktree_PreservesGroupName_WhenPromot Assert.Equal("maui", promoted.Name); } + [Fact] + public void PromoteOrCreateLocalFolderGroup_CreationPath_UsesFolderBasename() + { + // Documents intentional asymmetry: when no URL group exists to promote, + // the creation path names the group after the folder basename. + // This differs from promotion (which preserves the existing group name). + var svc = CreateService(); + // Note: path need not exist on disk; creation matches on repoId, not disk state. + var localRepoPath = Path.Combine(Path.GetTempPath(), "my-project"); + var expectedName = "my-project"; + + // No existing group for "new-repo" — creation path will be used + var result = svc.PromoteOrCreateLocalFolderGroup(localRepoPath, "new-repo"); + + Assert.True(result.IsLocalFolder); + Assert.Equal(expectedName, result.Name); + } + + [Fact] + public void PromoteOrCreateLocalFolderGroup_FallsBackToFolderName_WhenGroupNameIsEmpty() + { + // Defensive: if a promotable group has an empty name (corrupt state), + // promotion falls back to the folder basename instead of preserving blank. + var svc = CreateService(); + var localRepoPath = Path.Combine(Path.GetTempPath(), "fallback-name"); + var expectedPath = Path.GetFullPath(localRepoPath) + .TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); + + // Create a URL group and then blank out its name to simulate corrupt state + var urlGroup = svc.GetOrCreateRepoGroup("corrupt-repo", "original"); + Assert.NotNull(urlGroup); + urlGroup!.Name = ""; + + var result = svc.PromoteOrCreateLocalFolderGroup(localRepoPath, "corrupt-repo"); + + Assert.Equal(urlGroup.Id, result.Id); + Assert.True(result.IsLocalFolder); + Assert.Equal(expectedPath, result.LocalPath); + // Empty name should be repaired with the folder basename + Assert.Equal("fallback-name", result.Name); + } + [Fact] public void ReconcileOrganization_ExternalWorktree_PromotesUrlGroupToLocalFolderGroup() { diff --git a/PolyPilot/Services/CopilotService.Organization.cs b/PolyPilot/Services/CopilotService.Organization.cs index b6a7a9671..570b0d0d9 100644 --- a/PolyPilot/Services/CopilotService.Organization.cs +++ b/PolyPilot/Services/CopilotService.Organization.cs @@ -744,6 +744,9 @@ internal void ReconcileOrganization(bool allowPruning = true) // Preserve the user's group name — don't overwrite with the folder basename. // The old code did: groupToPromote.Name = Path.GetFileName(normalizedExtPath) // which destroyed user-customized names (e.g., "maui" → "maui2"). + // Fallback: if the group somehow has an empty name, use the folder basename. + if (string.IsNullOrWhiteSpace(groupToPromote.Name)) + groupToPromote.Name = Path.GetFileName(normalizedExtPath); changed = true; Debug($"ReconcileOrganization: promoted group '{groupToPromote.Id}' ('{groupToPromote.Name}') to local folder group for '{normalizedExtPath}'"); @@ -1538,6 +1541,9 @@ public SessionGroup PromoteOrCreateLocalFolderGroup(string localPath, string? re // Preserve the user's group name — don't overwrite with the folder basename. // The old code did: candidate.Name = Path.GetFileName(normalized) // which destroyed user-customized names (e.g., "maui" → "maui2"). + // Fallback: if the group somehow has an empty name, use the folder basename. + if (string.IsNullOrWhiteSpace(candidate.Name)) + candidate.Name = Path.GetFileName(normalized); SaveOrganization(); OnStateChanged?.Invoke(); Debug($"PromoteOrCreateLocalFolderGroup: promoted '{candidate.Id}' ('{candidate.Name}') to local folder group for '{normalized}'"); @@ -1546,6 +1552,10 @@ public SessionGroup PromoteOrCreateLocalFolderGroup(string localPath, string? re } // No existing group to promote — create a fresh local folder group. + // Note: the creation path uses Path.GetFileName(localPath) as the group name, + // which differs from promotion (which preserves the existing name). This is + // intentional: new groups get a sensible default from the folder name, while + // existing groups keep whatever the user (or auto-creation) named them. return GetOrCreateLocalFolderGroup(localPath, repoId); }