diff --git a/PolyPilot.Tests/RepoManagerTests.cs b/PolyPilot.Tests/RepoManagerTests.cs index 19998df1c3..3e8a539ce1 100644 --- a/PolyPilot.Tests/RepoManagerTests.cs +++ b/PolyPilot.Tests/RepoManagerTests.cs @@ -6,6 +6,16 @@ namespace PolyPilot.Tests; [Collection("BaseDir")] public class RepoManagerTests { + /// + /// Create the minimal files a bare git repo needs so IsValidBareRepository returns true. + /// + private static void CreateFakeBareRepoSkeleton(string bareDir) + { + Directory.CreateDirectory(bareDir); + Directory.CreateDirectory(Path.Combine(bareDir, "refs")); + File.WriteAllText(Path.Combine(bareDir, "HEAD"), "ref: refs/heads/main\n"); + } + [Theory] [InlineData("https://github.com/Owner/Repo.git", "Owner-Repo")] [InlineData("https://github.com/Owner/Repo", "Owner-Repo")] @@ -365,7 +375,7 @@ public void HealMissingRepos_DiscoversUntracked_BareClones() { // Create a fake bare clone directory with a git config var bareDir = Path.Combine(reposDir, "Owner-Repo.git"); - Directory.CreateDirectory(bareDir); + CreateFakeBareRepoSkeleton(bareDir); File.WriteAllText(Path.Combine(bareDir, "config"), "[remote \"origin\"]\n\turl = https://github.com/Owner/Repo\n\tfetch = +refs/heads/*:refs/remotes/origin/*\n"); @@ -458,7 +468,7 @@ public void HealMissingRepos_MultipleUntracked_AllDiscovered() foreach (var name in new[] { "dotnet-maui.git", "PureWeen-PolyPilot.git", "github-sdk.git" }) { var dir = Path.Combine(reposDir, name); - Directory.CreateDirectory(dir); + CreateFakeBareRepoSkeleton(dir); File.WriteAllText(Path.Combine(dir, "config"), $"[remote \"origin\"]\n\turl = https://github.com/test/{name.Replace(".git", "")}\n"); } @@ -501,7 +511,7 @@ public void Load_WithCorruptedState_HealsFromDisk() { // Create a bare clone on disk var bareDir = Path.Combine(reposDir, "Owner-Repo.git"); - Directory.CreateDirectory(bareDir); + CreateFakeBareRepoSkeleton(bareDir); File.WriteAllText(Path.Combine(bareDir, "config"), "[remote \"origin\"]\n\turl = https://github.com/Owner/Repo\n"); diff --git a/PolyPilot.Tests/WorktreeStrategyTests.cs b/PolyPilot.Tests/WorktreeStrategyTests.cs index d19523e14a..e147d0a554 100644 --- a/PolyPilot.Tests/WorktreeStrategyTests.cs +++ b/PolyPilot.Tests/WorktreeStrategyTests.cs @@ -769,4 +769,189 @@ public async Task FullyIsolated_WorktreeIdSetOnAgentSessionInfo() } #endregion + + #region Worktree Failure Fallback (Windows long-path / git failure scenarios) + + [Fact] + public async Task GroupShared_WorktreeFailure_FallsBackToExistingWorktree() + { + // Simulate scenario: worktree creation fails (e.g., Windows long-path issue) + // but an existing worktree for the repo exists — should use it instead of temp dir. + var wtDir = Path.Combine(Path.GetTempPath(), $"wt-fallback-test-{Guid.NewGuid():N}"); + Directory.CreateDirectory(wtDir); + try + { + var existingWt = new WorktreeInfo + { + Id = "existing-wt", + RepoId = "repo-1", + Branch = "main", + Path = wtDir + }; + var rm = new FailingRepoManagerWithExistingWorktree( + new() { new() { Id = "repo-1", Name = "Repo" } }, + new() { existingWt }); + var svc = CreateDemoService(rm); + var preset = MakePreset(2, WorktreeStrategy.GroupShared); + + var group = await svc.CreateGroupFromPresetAsync(preset, + workingDirectory: null, + repoId: "repo-1"); + + Assert.NotNull(group); + // Borrowed worktree should NOT set group.WorktreeId (prevents DeleteGroup from destroying it) + Assert.Null(group!.WorktreeId); + + // Session metadata WorktreeId must also be null — DeleteGroup collects from + // session metadata too, so borrowed IDs must not leak there. + var allMetas = svc.Organization.Sessions + .Where(s => s.GroupId == group!.Id) + .ToList(); + Assert.All(allMetas, m => Assert.Null(m.WorktreeId)); + + // Sessions should use the existing worktree path, not a temp dir + var organized = svc.GetOrganizedSessions(); + var groupSessions = organized.FirstOrDefault(g => g.Group.Id == group!.Id).Sessions; + Assert.NotNull(groupSessions); + Assert.All(groupSessions, s => + { + Assert.NotNull(s.WorkingDirectory); + Assert.Equal(wtDir, s.WorkingDirectory); + }); + } + finally + { + try { Directory.Delete(wtDir, true); } catch { } + } + } + + [Fact] + public async Task GroupShared_WorktreeFailure_WithWorkingDirectory_UsesWorkingDirectory() + { + // When worktree creation fails but a workingDirectory was provided, + // sessions should use the workingDirectory (not temp) + var rm = new FailingRepoManager(new() { new() { Id = "repo-1", Name = "Repo" } }); + var svc = CreateDemoService(rm); + var preset = MakePreset(2, WorktreeStrategy.GroupShared); + + var group = await svc.CreateGroupFromPresetAsync(preset, + workingDirectory: "/provided/fallback", + repoId: "repo-1"); + + Assert.NotNull(group); + + // orchWorkDir should still be /provided/fallback since worktree failed + var organized = svc.GetOrganizedSessions(); + var groupSessions = organized.FirstOrDefault(g => g.Group.Id == group!.Id).Sessions; + Assert.NotNull(groupSessions); + Assert.All(groupSessions, s => + { + Assert.NotNull(s.WorkingDirectory); + Assert.Equal("/provided/fallback", s.WorkingDirectory); + }); + } + + [Fact] + public async Task FullyIsolated_WorktreeFailure_FallsBackToExistingWorktree() + { + var wtDir = Path.Combine(Path.GetTempPath(), $"wt-fallback-test-{Guid.NewGuid():N}"); + Directory.CreateDirectory(wtDir); + try + { + var existingWt = new WorktreeInfo + { + Id = "existing-wt", + RepoId = "repo-1", + Branch = "main", + Path = wtDir + }; + var rm = new FailingRepoManagerWithExistingWorktree( + new() { new() { Id = "repo-1", Name = "Repo" } }, + new() { existingWt }); + var svc = CreateDemoService(rm); + var preset = MakePreset(2, WorktreeStrategy.FullyIsolated); + + var group = await svc.CreateGroupFromPresetAsync(preset, + workingDirectory: null, + repoId: "repo-1"); + + Assert.NotNull(group); + // Borrowed worktree should NOT set group.WorktreeId (prevents DeleteGroup from destroying it) + Assert.Null(group!.WorktreeId); + + // Session metadata WorktreeId must also be null — DeleteGroup collects from + // session metadata too, so borrowed IDs must not leak there. + Assert.All(svc.Organization.Sessions.Where(s => s.GroupId == group!.Id), + m => Assert.Null(m.WorktreeId)); + + // Sessions should still be created + var members = svc.Organization.Sessions + .Where(s => s.GroupId == group!.Id) + .ToList(); + Assert.Equal(3, members.Count); // 1 orch + 2 workers + + // All sessions should use the existing worktree path, not a temp dir + var organized = svc.GetOrganizedSessions(); + var groupSessions = organized.FirstOrDefault(g => g.Group.Id == group!.Id).Sessions; + Assert.NotNull(groupSessions); + Assert.All(groupSessions, s => + { + Assert.NotNull(s.WorkingDirectory); + Assert.Equal(wtDir, s.WorkingDirectory); + }); + } + finally + { + try { Directory.Delete(wtDir, true); } catch { } + } + } + + [Fact] + public async Task GroupShared_BranchName_UsesSharedPrefix() + { + // GroupShared should create a worktree with "-shared-" in the branch name, + // not "-orchestrator-" — this is the explicit handling fix. + var rm = new FakeRepoManager(new() { new() { Id = "repo-1", Name = "Repo" } }); + var svc = CreateDemoService(rm); + var preset = MakePreset(2, WorktreeStrategy.GroupShared); + + await svc.CreateGroupFromPresetAsync(preset, + workingDirectory: null, + repoId: "repo-1", + nameOverride: "MyTeam"); + + Assert.Single(rm.CreateCalls); + Assert.Contains("shared", rm.CreateCalls[0].BranchName); + Assert.DoesNotContain("orchestrator", rm.CreateCalls[0].BranchName); + } + + /// + /// A FailingRepoManager that also has existing worktrees in its state, + /// so the fallback logic can find them. + /// + private class FailingRepoManagerWithExistingWorktree : RepoManager + { + public FailingRepoManagerWithExistingWorktree(List repos, List worktrees) + { + 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(this, new RepositoryState { Repositories = repos, Worktrees = worktrees }); + loadedField.SetValue(this, true); + } + + public override Task CreateWorktreeAsync(string repoId, string branchName, + string? baseBranch = null, bool skipFetch = false, CancellationToken ct = default) + { + throw new InvalidOperationException("Simulated Windows long-path failure"); + } + + public override Task FetchAsync(string repoId, CancellationToken ct = default) + { + return Task.CompletedTask; // Fetch succeeds, only worktree creation fails + } + } + + #endregion } diff --git a/PolyPilot/Services/CopilotService.Organization.cs b/PolyPilot/Services/CopilotService.Organization.cs index 5c6daed58a..0018b45148 100644 --- a/PolyPilot/Services/CopilotService.Organization.cs +++ b/PolyPilot/Services/CopilotService.Organization.cs @@ -3622,14 +3622,15 @@ public string GetEffectiveModel(string sessionName) var orchWorkDir = workingDirectory; var orchWtId = worktreeId; - // Pre-fetch once to avoid parallel git lock contention (local mode only; server handles fetch in remote) - if (repoId != null && strategy != WorktreeStrategy.Shared && !IsRemoteMode) + // Pre-fetch once to avoid parallel git lock contention (local mode only; server handles fetch in remote). + // Shared and GroupShared fetch inside their own block below. + if (repoId != null && strategy != WorktreeStrategy.Shared && strategy != WorktreeStrategy.GroupShared && !IsRemoteMode) { try { await _repoManager.FetchAsync(repoId, ct); } catch (Exception ex) { Debug($"Pre-fetch failed (continuing): {ex.Message}"); } } - // For Shared strategy with a repo but no worktree, create a single shared worktree + // For Shared strategy with a repo but no worktree/dir, create a single shared worktree if (repoId != null && strategy == WorktreeStrategy.Shared && string.IsNullOrEmpty(worktreeId) && string.IsNullOrEmpty(workingDirectory)) { try @@ -3643,11 +3644,45 @@ public string GetEffectiveModel(string sessionName) } catch (Exception ex) { - Debug($"Failed to create shared worktree (sessions will use temp dirs): {ex.Message}"); + Debug($"[WorktreeStrategy] Failed to create shared worktree for strategy={strategy}, repoId={repoId}: {ex.GetType().Name}: {ex.Message}"); + orchWorkDir = TryGetExistingWorktreePath(repoId); + if (orchWorkDir != null) + Debug($"[WorktreeStrategy] Using existing worktree as fallback: {orchWorkDir}"); + else + Debug($"[WorktreeStrategy] No existing worktree found — sessions will use temp dirs"); } } - if (repoId != null && strategy != WorktreeStrategy.Shared && string.IsNullOrEmpty(worktreeId)) + // GroupShared: always create one shared worktree for the group (even if workingDirectory is set, + // because the group needs its own branch). Uses same naming as Shared. + if (repoId != null && strategy == WorktreeStrategy.GroupShared && string.IsNullOrEmpty(worktreeId)) + { + try + { + if (!IsRemoteMode) await _repoManager.FetchAsync(repoId, ct); + var sharedWt = await CreateWorktreeLocalOrRemoteAsync(repoId, $"{branchPrefix}-shared-{Guid.NewGuid().ToString()[..4]}", ct); + orchWorkDir = sharedWt.Path; + orchWtId = sharedWt.Id; + group.WorktreeId = orchWtId; + group.CreatedWorktreeIds.Add(orchWtId); + } + catch (Exception ex) + { + Debug($"[WorktreeStrategy] Failed to create shared worktree for strategy={strategy}, repoId={repoId}: {ex.GetType().Name}: {ex.Message}"); + // Try to fall back to an existing worktree for this repo instead of temp dir + if (orchWorkDir == null) + { + orchWorkDir = TryGetExistingWorktreePath(repoId); + if (orchWorkDir != null) + Debug($"[WorktreeStrategy] Using existing worktree as fallback: {orchWorkDir}"); + else + Debug($"[WorktreeStrategy] No existing worktree found — sessions will use temp dirs"); + } + } + } + + // OrchestratorIsolated / FullyIsolated: create a dedicated orchestrator worktree + if (repoId != null && strategy != WorktreeStrategy.Shared && strategy != WorktreeStrategy.GroupShared && string.IsNullOrEmpty(worktreeId)) { try { @@ -3659,7 +3694,15 @@ public string GetEffectiveModel(string sessionName) } catch (Exception ex) { - Debug($"Failed to create orchestrator worktree (falling back to shared): {ex.Message}"); + Debug($"[WorktreeStrategy] Failed to create orchestrator worktree for strategy={strategy}, repoId={repoId}: {ex.GetType().Name}: {ex.Message}"); + if (orchWorkDir == null) + { + orchWorkDir = TryGetExistingWorktreePath(repoId); + if (orchWorkDir != null) + Debug($"[WorktreeStrategy] Using existing worktree as fallback: {orchWorkDir}"); + else + Debug($"[WorktreeStrategy] No existing worktree found — sessions will use temp dirs"); + } } } @@ -3679,7 +3722,8 @@ public string GetEffectiveModel(string sessionName) } catch (Exception ex) { - Debug($"Failed to create worker-{i + 1} worktree (falling back to shared): {ex.Message}"); + Debug($"[WorktreeStrategy] Failed to create worker-{i + 1} worktree: {ex.GetType().Name}: {ex.Message}"); + Debug($"[WorktreeStrategy] Worker-{i + 1} will fall back to orchestrator dir: {orchWorkDir}"); } } } @@ -3697,7 +3741,7 @@ public string GetEffectiveModel(string sessionName) } catch (Exception ex) { - Debug($"Failed to create shared worker worktree (falling back to shared): {ex.Message}"); + Debug($"[WorktreeStrategy] Failed to create shared worker worktree: {ex.GetType().Name}: {ex.Message}"); } } @@ -4681,5 +4725,18 @@ private void AutoAdjustFromFeedback(string groupId, SessionGroup group, List + /// When worktree creation fails (e.g., long paths on Windows), try to find an existing + /// worktree for the repo so sessions get a real working directory instead of a temp dir. + /// Returns only the path — does NOT propagate the worktree ID. This prevents the borrowed + /// ID from leaking into session metadata, which would cause DeleteGroup to destroy a + /// worktree that belongs to another group (DeleteGroup collects IDs from session metadata). + /// + private string? TryGetExistingWorktreePath(string repoId) + { + var existing = _repoManager.Worktrees.FirstOrDefault(w => w.RepoId == repoId && Directory.Exists(w.Path)); + return existing?.Path; + } + #endregion }