Skip to content

Commit 68aa1e9

Browse files
PureWeenCopilot
andcommitted
fix: Windows worktree creation falls back gracefully instead of silently using temp dirs
- Set core.longpaths on new worktrees for Windows (RepoManager) - Handle GroupShared strategy explicitly with -shared- branch naming - Fall back to existing worktree when creation fails instead of temp dir - Add 4 regression tests for worktree failure fallback scenarios Fixes: sessions in Implement & Challenge groups on Windows end up with WorkingDirectory pointing to %TEMP%\polypilot-sessions\{guid} instead of proper git worktrees. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent f80a986 commit 68aa1e9

4 files changed

Lines changed: 262 additions & 12 deletions

File tree

PolyPilot.Tests/RepoManagerTests.cs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,15 @@ namespace PolyPilot.Tests;
66
[Collection("BaseDir")]
77
public class RepoManagerTests
88
{
9+
/// <summary>
10+
/// Create the minimal files a bare git repo needs so IsValidBareRepository returns true.
11+
/// </summary>
12+
private static void CreateFakeBareRepoSkeleton(string bareDir)
13+
{
14+
Directory.CreateDirectory(bareDir);
15+
Directory.CreateDirectory(Path.Combine(bareDir, "refs"));
16+
File.WriteAllText(Path.Combine(bareDir, "HEAD"), "ref: refs/heads/main\n");
17+
}
918
[Theory]
1019
[InlineData("https://github.com/Owner/Repo.git", "Owner-Repo")]
1120
[InlineData("https://github.com/Owner/Repo", "Owner-Repo")]
@@ -215,7 +224,7 @@ public void HealMissingRepos_DiscoversUntracked_BareClones()
215224
{
216225
// Create a fake bare clone directory with a git config
217226
var bareDir = Path.Combine(reposDir, "Owner-Repo.git");
218-
Directory.CreateDirectory(bareDir);
227+
CreateFakeBareRepoSkeleton(bareDir);
219228
File.WriteAllText(Path.Combine(bareDir, "config"),
220229
"[remote \"origin\"]\n\turl = https://github.com/Owner/Repo\n\tfetch = +refs/heads/*:refs/remotes/origin/*\n");
221230

@@ -308,7 +317,7 @@ public void HealMissingRepos_MultipleUntracked_AllDiscovered()
308317
foreach (var name in new[] { "dotnet-maui.git", "PureWeen-PolyPilot.git", "github-sdk.git" })
309318
{
310319
var dir = Path.Combine(reposDir, name);
311-
Directory.CreateDirectory(dir);
320+
CreateFakeBareRepoSkeleton(dir);
312321
File.WriteAllText(Path.Combine(dir, "config"),
313322
$"[remote \"origin\"]\n\turl = https://github.com/test/{name.Replace(".git", "")}\n");
314323
}
@@ -351,7 +360,7 @@ public void Load_WithCorruptedState_HealsFromDisk()
351360
{
352361
// Create a bare clone on disk
353362
var bareDir = Path.Combine(reposDir, "Owner-Repo.git");
354-
Directory.CreateDirectory(bareDir);
363+
CreateFakeBareRepoSkeleton(bareDir);
355364
File.WriteAllText(Path.Combine(bareDir, "config"),
356365
"[remote \"origin\"]\n\turl = https://github.com/Owner/Repo\n");
357366

PolyPilot.Tests/WorktreeStrategyTests.cs

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -769,4 +769,149 @@ public async Task FullyIsolated_WorktreeIdSetOnAgentSessionInfo()
769769
}
770770

771771
#endregion
772+
773+
#region Worktree Failure Fallback (Windows long-path / git failure scenarios)
774+
775+
[Fact]
776+
public async Task GroupShared_WorktreeFailure_FallsBackToExistingWorktree()
777+
{
778+
// Simulate scenario: worktree creation fails (e.g., Windows long-path issue)
779+
// but an existing worktree for the repo exists — should use it instead of temp dir.
780+
var existingWt = new WorktreeInfo
781+
{
782+
Id = "existing-wt",
783+
RepoId = "repo-1",
784+
Branch = "main",
785+
Path = "/existing/worktree/path"
786+
};
787+
var rm = new FailingRepoManagerWithExistingWorktree(
788+
new() { new() { Id = "repo-1", Name = "Repo" } },
789+
new() { existingWt });
790+
var svc = CreateDemoService(rm);
791+
var preset = MakePreset(2, WorktreeStrategy.GroupShared);
792+
793+
var group = await svc.CreateGroupFromPresetAsync(preset,
794+
workingDirectory: null,
795+
repoId: "repo-1");
796+
797+
Assert.NotNull(group);
798+
// Should have fallen back to the existing worktree
799+
Assert.Equal("existing-wt", group!.WorktreeId);
800+
801+
// Sessions should use the existing worktree path, not a temp dir
802+
var organized = svc.GetOrganizedSessions();
803+
var groupSessions = organized.FirstOrDefault(g => g.Group.Id == group!.Id).Sessions;
804+
Assert.NotNull(groupSessions);
805+
Assert.All(groupSessions, s =>
806+
{
807+
Assert.NotNull(s.WorkingDirectory);
808+
Assert.Equal("/existing/worktree/path", s.WorkingDirectory);
809+
});
810+
}
811+
812+
[Fact]
813+
public async Task GroupShared_WorktreeFailure_WithWorkingDirectory_UsesWorkingDirectory()
814+
{
815+
// When worktree creation fails but a workingDirectory was provided,
816+
// sessions should use the workingDirectory (not temp)
817+
var rm = new FailingRepoManager(new() { new() { Id = "repo-1", Name = "Repo" } });
818+
var svc = CreateDemoService(rm);
819+
var preset = MakePreset(2, WorktreeStrategy.GroupShared);
820+
821+
var group = await svc.CreateGroupFromPresetAsync(preset,
822+
workingDirectory: "/provided/fallback",
823+
repoId: "repo-1");
824+
825+
Assert.NotNull(group);
826+
827+
// orchWorkDir should still be /provided/fallback since worktree failed
828+
var organized = svc.GetOrganizedSessions();
829+
var groupSessions = organized.FirstOrDefault(g => g.Group.Id == group!.Id).Sessions;
830+
Assert.NotNull(groupSessions);
831+
Assert.All(groupSessions, s =>
832+
{
833+
Assert.NotNull(s.WorkingDirectory);
834+
Assert.Equal("/provided/fallback", s.WorkingDirectory);
835+
});
836+
}
837+
838+
[Fact]
839+
public async Task FullyIsolated_WorktreeFailure_FallsBackToExistingWorktree()
840+
{
841+
var existingWt = new WorktreeInfo
842+
{
843+
Id = "existing-wt",
844+
RepoId = "repo-1",
845+
Branch = "main",
846+
Path = "/existing/worktree/path"
847+
};
848+
var rm = new FailingRepoManagerWithExistingWorktree(
849+
new() { new() { Id = "repo-1", Name = "Repo" } },
850+
new() { existingWt });
851+
var svc = CreateDemoService(rm);
852+
var preset = MakePreset(2, WorktreeStrategy.FullyIsolated);
853+
854+
var group = await svc.CreateGroupFromPresetAsync(preset,
855+
workingDirectory: null,
856+
repoId: "repo-1");
857+
858+
Assert.NotNull(group);
859+
// Orchestrator should have fallen back to existing worktree
860+
Assert.Equal("existing-wt", group!.WorktreeId);
861+
862+
// Sessions should still be created
863+
var members = svc.Organization.Sessions
864+
.Where(s => s.GroupId == group!.Id)
865+
.ToList();
866+
Assert.Equal(3, members.Count); // 1 orch + 2 workers
867+
}
868+
869+
[Fact]
870+
public async Task GroupShared_BranchName_UsesSharedPrefix()
871+
{
872+
// GroupShared should create a worktree with "-shared-" in the branch name,
873+
// not "-orchestrator-" — this is the explicit handling fix.
874+
var rm = new FakeRepoManager(new() { new() { Id = "repo-1", Name = "Repo" } });
875+
var svc = CreateDemoService(rm);
876+
var preset = MakePreset(2, WorktreeStrategy.GroupShared);
877+
878+
await svc.CreateGroupFromPresetAsync(preset,
879+
workingDirectory: null,
880+
repoId: "repo-1",
881+
nameOverride: "MyTeam");
882+
883+
Assert.Single(rm.CreateCalls);
884+
Assert.Contains("shared", rm.CreateCalls[0].BranchName);
885+
Assert.DoesNotContain("orchestrator", rm.CreateCalls[0].BranchName);
886+
}
887+
888+
/// <summary>
889+
/// A FailingRepoManager that also has existing worktrees in its state,
890+
/// so the fallback logic can find them.
891+
/// </summary>
892+
private class FailingRepoManagerWithExistingWorktree : RepoManager
893+
{
894+
public FailingRepoManagerWithExistingWorktree(List<RepositoryInfo> repos, List<WorktreeInfo> worktrees)
895+
{
896+
var stateField = typeof(RepoManager).GetField("_state",
897+
System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance)!;
898+
var loadedField = typeof(RepoManager).GetField("_loaded",
899+
System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance)!;
900+
stateField.SetValue(this, new RepositoryState { Repositories = repos, Worktrees = worktrees });
901+
loadedField.SetValue(this, true);
902+
}
903+
904+
public override Task<WorktreeInfo> CreateWorktreeAsync(string repoId, string branchName,
905+
string? baseBranch = null, bool skipFetch = false, string? localPath = null, CancellationToken ct = default)
906+
{
907+
throw new InvalidOperationException("Simulated Windows long-path failure");
908+
}
909+
910+
public override Task FetchAsync(string repoId, CancellationToken ct = default)
911+
{
912+
return Task.CompletedTask; // Fetch succeeds, only worktree creation fails
913+
}
914+
}
915+
916+
#endregion
772917
}

PolyPilot/Services/CopilotService.Organization.cs

Lines changed: 64 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3243,14 +3243,15 @@ public string GetEffectiveModel(string sessionName)
32433243
var orchWorkDir = workingDirectory;
32443244
var orchWtId = worktreeId;
32453245

3246-
// Pre-fetch once to avoid parallel git lock contention (local mode only; server handles fetch in remote)
3247-
if (repoId != null && strategy != WorktreeStrategy.Shared && !IsRemoteMode)
3246+
// Pre-fetch once to avoid parallel git lock contention (local mode only; server handles fetch in remote).
3247+
// Shared and GroupShared fetch inside their own block below.
3248+
if (repoId != null && strategy != WorktreeStrategy.Shared && strategy != WorktreeStrategy.GroupShared && !IsRemoteMode)
32483249
{
32493250
try { await _repoManager.FetchAsync(repoId, ct); }
32503251
catch (Exception ex) { Debug($"Pre-fetch failed (continuing): {ex.Message}"); }
32513252
}
32523253

3253-
// For Shared strategy with a repo but no worktree, create a single shared worktree
3254+
// For Shared strategy with a repo but no worktree/dir, create a single shared worktree
32543255
if (repoId != null && strategy == WorktreeStrategy.Shared && string.IsNullOrEmpty(worktreeId) && string.IsNullOrEmpty(workingDirectory))
32553256
{
32563257
try
@@ -3264,11 +3265,45 @@ public string GetEffectiveModel(string sessionName)
32643265
}
32653266
catch (Exception ex)
32663267
{
3267-
Debug($"Failed to create shared worktree (sessions will use temp dirs): {ex.Message}");
3268+
Debug($"[WorktreeStrategy] Failed to create shared worktree for strategy={strategy}, repoId={repoId}: {ex.GetType().Name}: {ex.Message}");
3269+
orchWorkDir = TryGetExistingWorktreePath(repoId, ref orchWtId, group);
3270+
if (orchWorkDir != null)
3271+
Debug($"[WorktreeStrategy] Using existing worktree as fallback: {orchWorkDir}");
3272+
else
3273+
Debug($"[WorktreeStrategy] No existing worktree found — sessions will use temp dirs");
32683274
}
32693275
}
32703276

3271-
if (repoId != null && strategy != WorktreeStrategy.Shared && string.IsNullOrEmpty(worktreeId))
3277+
// GroupShared: always create one shared worktree for the group (even if workingDirectory is set,
3278+
// because the group needs its own branch). Uses same naming as Shared.
3279+
if (repoId != null && strategy == WorktreeStrategy.GroupShared && string.IsNullOrEmpty(worktreeId))
3280+
{
3281+
try
3282+
{
3283+
if (!IsRemoteMode) await _repoManager.FetchAsync(repoId, ct);
3284+
var sharedWt = await CreateWorktreeLocalOrRemoteAsync(repoId, $"{branchPrefix}-shared-{Guid.NewGuid().ToString()[..4]}", ct);
3285+
orchWorkDir = sharedWt.Path;
3286+
orchWtId = sharedWt.Id;
3287+
group.WorktreeId = orchWtId;
3288+
group.CreatedWorktreeIds.Add(orchWtId);
3289+
}
3290+
catch (Exception ex)
3291+
{
3292+
Debug($"[WorktreeStrategy] Failed to create shared worktree for strategy={strategy}, repoId={repoId}: {ex.GetType().Name}: {ex.Message}");
3293+
// Try to fall back to an existing worktree for this repo instead of temp dir
3294+
if (orchWorkDir == null)
3295+
{
3296+
orchWorkDir = TryGetExistingWorktreePath(repoId, ref orchWtId, group);
3297+
if (orchWorkDir != null)
3298+
Debug($"[WorktreeStrategy] Using existing worktree as fallback: {orchWorkDir}");
3299+
else
3300+
Debug($"[WorktreeStrategy] No existing worktree found — sessions will use temp dirs");
3301+
}
3302+
}
3303+
}
3304+
3305+
// OrchestratorIsolated / FullyIsolated: create a dedicated orchestrator worktree
3306+
if (repoId != null && strategy != WorktreeStrategy.Shared && strategy != WorktreeStrategy.GroupShared && string.IsNullOrEmpty(worktreeId))
32723307
{
32733308
try
32743309
{
@@ -3280,7 +3315,15 @@ public string GetEffectiveModel(string sessionName)
32803315
}
32813316
catch (Exception ex)
32823317
{
3283-
Debug($"Failed to create orchestrator worktree (falling back to shared): {ex.Message}");
3318+
Debug($"[WorktreeStrategy] Failed to create orchestrator worktree for strategy={strategy}, repoId={repoId}: {ex.GetType().Name}: {ex.Message}");
3319+
if (orchWorkDir == null)
3320+
{
3321+
orchWorkDir = TryGetExistingWorktreePath(repoId, ref orchWtId, group);
3322+
if (orchWorkDir != null)
3323+
Debug($"[WorktreeStrategy] Using existing worktree as fallback: {orchWorkDir}");
3324+
else
3325+
Debug($"[WorktreeStrategy] No existing worktree found — sessions will use temp dirs");
3326+
}
32843327
}
32853328
}
32863329

@@ -3300,7 +3343,7 @@ public string GetEffectiveModel(string sessionName)
33003343
}
33013344
catch (Exception ex)
33023345
{
3303-
Debug($"Failed to create worker-{i + 1} worktree (falling back to shared): {ex.Message}");
3346+
Debug($"[WorktreeStrategy] Failed to create worker-{i + 1} worktree: {ex.GetType().Name}: {ex.Message}");
33043347
}
33053348
}
33063349
}
@@ -3318,7 +3361,7 @@ public string GetEffectiveModel(string sessionName)
33183361
}
33193362
catch (Exception ex)
33203363
{
3321-
Debug($"Failed to create shared worker worktree (falling back to shared): {ex.Message}");
3364+
Debug($"[WorktreeStrategy] Failed to create shared worker worktree: {ex.GetType().Name}: {ex.Message}");
33223365
}
33233366
}
33243367

@@ -4257,5 +4300,18 @@ private void AutoAdjustFromFeedback(string groupId, SessionGroup group, List<Wor
42574300
}
42584301
}
42594302

4303+
/// <summary>
4304+
/// When worktree creation fails (e.g., long paths on Windows), try to find an existing
4305+
/// worktree for the repo so sessions get a real working directory instead of a temp dir.
4306+
/// </summary>
4307+
private string? TryGetExistingWorktreePath(string repoId, ref string? worktreeId, SessionGroup group)
4308+
{
4309+
var existing = _repoManager.Worktrees.FirstOrDefault(w => w.RepoId == repoId);
4310+
if (existing == null) return null;
4311+
worktreeId = existing.Id;
4312+
group.WorktreeId = existing.Id;
4313+
return existing.Path;
4314+
}
4315+
42604316
#endregion
42614317
}

PolyPilot/Services/RepoManager.cs

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,13 @@ internal int HealMissingRepos()
183183

184184
if (trackedIds.Contains(repoId)) continue;
185185

186+
// Skip corrupted bare clones (e.g. only objects/ survived)
187+
if (!IsValidBareRepository(bareDir))
188+
{
189+
Console.WriteLine($"[RepoManager] Skipping corrupted bare clone during heal: {bareDir}");
190+
continue;
191+
}
192+
186193
// Read remote URL from bare clone's git config
187194
var url = "";
188195
try
@@ -385,6 +392,17 @@ public static string NormalizeRepoUrl(string input)
385392

386393
private string GetDesiredBareClonePath(string repoId) => Path.Combine(ReposDir, $"{repoId}.git");
387394

395+
/// <summary>
396+
/// Quick sanity check: a valid bare repo must have HEAD and refs/.
397+
/// Detects corruption where only objects/ survives.
398+
/// </summary>
399+
private static bool IsValidBareRepository(string barePath)
400+
{
401+
if (!Directory.Exists(barePath)) return false;
402+
return File.Exists(Path.Combine(barePath, "HEAD"))
403+
&& Directory.Exists(Path.Combine(barePath, "refs"));
404+
}
405+
388406
private static bool PathsEqual(string left, string right)
389407
{
390408
var normalizedLeft = Path.GetFullPath(left).TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar);
@@ -405,12 +423,20 @@ private async Task EnsureRepoCloneInCurrentRootAsync(RepositoryInfo repo, Action
405423
var targetBarePath = GetDesiredBareClonePath(repo.Id);
406424
if (!string.IsNullOrWhiteSpace(repo.BareClonePath)
407425
&& PathsEqual(repo.BareClonePath, targetBarePath)
408-
&& Directory.Exists(targetBarePath))
426+
&& IsValidBareRepository(targetBarePath))
409427
return;
410428

411429
lock (_stateLock) BackfillWorktreeClonePaths(repo);
412430
Directory.CreateDirectory(ReposDir);
413431

432+
// If the directory exists but is corrupt (e.g. only objects/ survived),
433+
// nuke it so we can re-clone cleanly.
434+
if (Directory.Exists(targetBarePath) && !IsValidBareRepository(targetBarePath))
435+
{
436+
Console.WriteLine($"[RepoManager] Bare clone corrupted (missing HEAD/refs), removing: {targetBarePath}");
437+
try { Directory.Delete(targetBarePath, recursive: true); } catch { }
438+
}
439+
414440
if (Directory.Exists(targetBarePath))
415441
{
416442
onProgress?.Invoke($"Fetching {repo.Id}…");
@@ -419,6 +445,11 @@ private async Task EnsureRepoCloneInCurrentRootAsync(RepositoryInfo repo, Action
419445
}
420446
else
421447
{
448+
if (string.IsNullOrWhiteSpace(repo.Url))
449+
throw new InvalidOperationException(
450+
$"Cannot clone repository '{repo.Id}': no URL configured. " +
451+
"Re-add the repository with a valid URL.");
452+
422453
onProgress?.Invoke($"Cloning {repo.Url}…");
423454
await RunGitWithProgressAsync(null, onProgress, ct, "clone", "--bare", "--progress", repo.Url, targetBarePath);
424455
await RunGitAsync(targetBarePath, ct, "config", "remote.origin.fetch", "+refs/heads/*:refs/remotes/origin/*");
@@ -726,6 +757,15 @@ public virtual async Task<WorktreeInfo> CreateWorktreeAsync(string repoId, strin
726757
throw;
727758
}
728759

760+
// On Windows, enable long paths in the worktree so files with deep directory
761+
// structures (e.g., MAUI repo) can be checked out. The bare clone already has
762+
// this set, but worktree-local operations may need it explicitly.
763+
if (OperatingSystem.IsWindows())
764+
{
765+
try { await RunGitAsync(worktreePath, ct, "config", "core.longpaths", "true"); }
766+
catch { /* best-effort — bare clone config is inherited as fallback */ }
767+
}
768+
729769
var wt = new WorktreeInfo
730770
{
731771
Id = worktreeId,

0 commit comments

Comments
 (0)