Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions PolyPilot.Tests/RepoManagerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,16 @@ namespace PolyPilot.Tests;
[Collection("BaseDir")]
public class RepoManagerTests
{
/// <summary>
/// Create the minimal files a bare git repo needs so IsValidBareRepository returns true.
/// </summary>
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")]
Expand Down Expand Up @@ -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");

Expand Down Expand Up @@ -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");
}
Expand Down Expand Up @@ -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");

Expand Down
185 changes: 185 additions & 0 deletions PolyPilot.Tests/WorktreeStrategyTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/// <summary>
/// A FailingRepoManager that also has existing worktrees in its state,
/// so the fallback logic can find them.
/// </summary>
private class FailingRepoManagerWithExistingWorktree : RepoManager
{
public FailingRepoManagerWithExistingWorktree(List<RepositoryInfo> repos, List<WorktreeInfo> 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<WorktreeInfo> 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
}
73 changes: 65 additions & 8 deletions PolyPilot/Services/CopilotService.Organization.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
{
Expand All @@ -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");
}
}
}

Expand All @@ -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}");
}
}
}
Expand All @@ -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}");
}
}

Expand Down Expand Up @@ -4681,5 +4725,18 @@ private void AutoAdjustFromFeedback(string groupId, SessionGroup group, List<Wor
}
}

/// <summary>
/// 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).
/// </summary>
private string? TryGetExistingWorktreePath(string repoId)
{
var existing = _repoManager.Worktrees.FirstOrDefault(w => w.RepoId == repoId && Directory.Exists(w.Path));
return existing?.Path;
}

#endregion
}