Skip to content
Open
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
71 changes: 47 additions & 24 deletions PolyPilot.Tests/AddExistingRepoTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -189,11 +189,10 @@ public void Reconcile_SessionInDefault_WithOnlyUrlGroup_FallsBackToUrlGroup()
// ─── Bug 1: AddRepositoryAsync supports local clone source ─────────────────

[Fact]
public async Task AddRepositoryFromLocal_ClonesLocallyAndSetsRemoteUrl()
public async Task AddRepositoryFromLocal_PointsBareClonePathAtLocalRepo()
{
// Create a real local git repo with an origin remote, then call
// AddRepositoryFromLocalAsync and verify the bare clone's remote URL
// is the network URL (not the local path).
// AddRepositoryFromLocalAsync should set BareClonePath to the local path
// (no bare clone is created) and register the repo.
var tempDir = Path.Combine(Path.GetTempPath(), $"local-clone-test-{Guid.NewGuid():N}");
var testBaseDir = Path.Combine(Path.GetTempPath(), $"rmtest-{Guid.NewGuid():N}");
Directory.CreateDirectory(tempDir);
Expand All @@ -212,16 +211,15 @@ public async Task AddRepositoryFromLocal_ClonesLocallyAndSetsRemoteUrl()
RepoManager.SetBaseDirForTesting(testBaseDir);
try
{
var progressMessages = new List<string>();
var repo = await rm.AddRepositoryFromLocalAsync(
tempDir, msg => progressMessages.Add(msg));
var repo = await rm.AddRepositoryFromLocalAsync(tempDir);

// Should have used local clone, not network
Assert.Contains(progressMessages, m => m.Contains("local folder", StringComparison.OrdinalIgnoreCase));
// BareClonePath should point at the user's local repo — no bare clone
Assert.Equal(Path.GetFullPath(tempDir), Path.GetFullPath(repo.BareClonePath));

// The bare clone's remote origin should point to the network URL
var bareRemoteUrl = await RunGitOutput(repo.BareClonePath, "remote", "get-url", "origin");
Assert.Equal(remoteUrl, bareRemoteUrl.Trim());
// No bare clone directory should exist under the managed repos dir
var reposDir = Path.Combine(testBaseDir, "repos");
if (Directory.Exists(reposDir))
Assert.Empty(Directory.GetDirectories(reposDir));

// Verify the repo was registered
Assert.Contains(rm.Repositories, r => r.Id == repo.Id);
Expand All @@ -239,20 +237,37 @@ public async Task AddRepositoryFromLocal_ClonesLocallyAndSetsRemoteUrl()
}

[Fact]
public async Task AddRepositoryAsync_LocalCloneSource_InvalidPath_Throws()
public void AddRepositoryFromLocal_NoBareCloneCreatedInReposDir()
{
var rm = new RepoManager();
var method = typeof(RepoManager).GetMethod("AddRepositoryAsync",
System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance,
null,
new[] { typeof(string), typeof(Action<string>), typeof(string), typeof(CancellationToken) },
null)!;

var ex = await Assert.ThrowsAsync<ArgumentException>(async () =>
await (Task<RepositoryInfo>)method.Invoke(rm,
new object?[] { "https://github.com/test/repo", null, "/nonexistent/path", CancellationToken.None })!);
// Verify that AddRepositoryFromLocalAsync does NOT call AddRepositoryAsync
// (which would create a bare clone). Our approach sets BareClonePath directly
// to the local path — the internal localCloneSource overload is no longer used.
var sourceFile = File.ReadAllText(Path.Combine(GetRepoRoot(), "PolyPilot", "Services", "RepoManager.cs"));

// AddRepositoryFromLocalAsync should NOT call AddRepositoryAsync
// Instead it should directly create a RepositoryInfo with BareClonePath = localPath
var methodBody = ExtractMethodBody(sourceFile, "AddRepositoryFromLocalAsync");
Assert.DoesNotContain("AddRepositoryAsync(", methodBody);
Assert.Contains("BareClonePath = localPath", methodBody);
}

Assert.Contains("not found", ex.Message, StringComparison.OrdinalIgnoreCase);
private static string ExtractMethodBody(string source, string methodName)
{
var idx = source.IndexOf(methodName, StringComparison.Ordinal);
if (idx < 0) return "";
// Find opening brace
var braceIdx = source.IndexOf('{', idx);
if (braceIdx < 0) return "";
// Find matching closing brace
var depth = 1;
var i = braceIdx + 1;
while (i < source.Length && depth > 0)
{
if (source[i] == '{') depth++;
else if (source[i] == '}') depth--;
i++;
}
return source[braceIdx..i];
}

// ─── Bug 2 (second block): WorktreeId-based reconcile prefers local folder ─
Expand Down Expand Up @@ -351,4 +366,12 @@ private static void ForceDeleteDirectory(string path)
File.SetAttributes(f, FileAttributes.Normal);
Directory.Delete(path, true);
}

private static string GetRepoRoot()
{
var dir = new DirectoryInfo(AppContext.BaseDirectory);
while (dir != null && !File.Exists(Path.Combine(dir.FullName, "PolyPilot.slnx")))
dir = dir.Parent;
return dir?.FullName ?? throw new InvalidOperationException("Could not find repo root");
}
}
65 changes: 65 additions & 0 deletions PolyPilot.Tests/ProcessingWatchdogTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3096,4 +3096,69 @@ public void WatchdogCaseB_UsesFileInfoForSizeAndTime_InSource()
Assert.True(watchdogBody.Contains("fileInfo.Length"),
"Case B must read Length from FileInfo");
}

[Fact]
public void PollMaxStaleCycles_IsReasonable()
{
// PollMaxStaleCycles × 5s poll interval = total staleness wait before force-reconnect.
// 12 × 5s = 60s — long enough to avoid false triggers on legitimate long-running tools
// (where events.jsonl pauses between tool.execution_start and tool.execution_complete),
// but short enough to detect stuck sessions quickly after app restart.
Assert.Equal(12, CopilotService.PollMaxStaleCycles);
Assert.True(CopilotService.PollMaxStaleCycles >= 6,
"Below 6 cycles (30s) risks false-positive staleness on normal tool execution gaps");
Assert.True(CopilotService.PollMaxStaleCycles <= 24,
"Above 24 cycles (120s) delays stuck-session recovery unnecessarily");
}

[Fact]
public void PollStalenessDetection_ForceReconnect_InSource()
{
// The poller must detect when events.jsonl hasn't grown and force a reconnect
// via EnsureSessionConnectedAsync. This prevents the 600s watchdog delay when
// the CLI server's session is stuck after client disconnect.
var source = File.ReadAllText(
Path.Combine(GetRepoRoot(), "PolyPilot", "Services", "CopilotService.Persistence.cs"));
var methodIdx = source.IndexOf("private async Task PollEventsAndResumeWhenIdleAsync");
Assert.True(methodIdx >= 0, "PollEventsAndResumeWhenIdleAsync must exist");
var endIdx = source.IndexOf("\n }", methodIdx);
var pollerBody = source.Substring(methodIdx, endIdx - methodIdx);

// Must track file size for staleness detection
Assert.True(pollerBody.Contains("initialFileSize"),
"Poller must track initial file size for staleness detection");
Assert.True(pollerBody.Contains("staleCycles"),
"Poller must count consecutive stale poll cycles");
Assert.True(pollerBody.Contains("PollMaxStaleCycles"),
"Poller must use PollMaxStaleCycles constant for threshold");

// Must force-connect when stale
Assert.True(pollerBody.Contains("EnsureSessionConnectedAsync"),
"Poller must call EnsureSessionConnectedAsync when file is stale");
Assert.True(pollerBody.Contains("[POLL-STALE]"),
"Poller must log [POLL-STALE] tag for diagnostics");

// After force-connect, must clear HasUsedToolsThisTurn so watchdog uses 30s quiescence
Assert.True(pollerBody.Contains("HasUsedToolsThisTurn = false"),
"Poller must clear HasUsedToolsThisTurn after force-reconnect to enable 30s quiescence");
}

[Fact]
public void PollStalenessDetection_ResetsOnGrowth_InSource()
{
// When events.jsonl grows, the poller must reset its staleness tracking.
// This prevents false-positive force-reconnects on sessions where the CLI
// is actively running tools (file grows between poll cycles).
var source = File.ReadAllText(
Path.Combine(GetRepoRoot(), "PolyPilot", "Services", "CopilotService.Persistence.cs"));
var methodIdx = source.IndexOf("private async Task PollEventsAndResumeWhenIdleAsync");
var endIdx = source.IndexOf("\n }", methodIdx);
var pollerBody = source.Substring(methodIdx, endIdx - methodIdx);

// Must reset staleness counters when file grows
Assert.True(pollerBody.Contains("staleCycles = 0"),
"Poller must reset staleCycles when file grows");
Assert.True(pollerBody.Contains("initialFileSize = currentSize"),
"Poller must update baseline file size when file grows");
}
}
101 changes: 61 additions & 40 deletions PolyPilot.Tests/RepoManagerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -997,35 +997,10 @@ public async Task RemoveWorktreeAsync_NoBareClone_ExternalPath_DoesNotDeleteDire
#region CreateWorktreeAsync Path Strategy Tests

[Fact]
public void CreateWorktree_WithLocalPath_PlacesWorktreeInsideLocalRepo()
public void CreateWorktree_AlwaysPlacesWorktreeInCentralDir()
{
// When localPath is provided, the worktree path should be:
// {localPath}/.polypilot/worktrees/{branchName}
// This is the "nested strategy" that keeps worktrees inside the user's repo.

var localRepoPath = Path.Combine(Path.GetTempPath(), "my-local-repo");
var branchName = "feature-login";
var repoWorktreesDir = Path.Combine(localRepoPath, ".polypilot", "worktrees");
var expectedPath = Path.Combine(repoWorktreesDir, branchName);
var resolved = Path.GetFullPath(expectedPath);
var managedBase = Path.GetFullPath(repoWorktreesDir) + Path.DirectorySeparatorChar;

// Verify path is inside the managed dir (passes the guard)
Assert.True(resolved.StartsWith(managedBase, StringComparison.OrdinalIgnoreCase),
$"Expected path '{resolved}' to be inside '{managedBase}'");

// Verify it is NOT under the centralized worktrees dir
var centralDir = Path.Combine(Path.GetTempPath(), ".polypilot", "worktrees");
Assert.False(resolved.StartsWith(Path.GetFullPath(centralDir), StringComparison.OrdinalIgnoreCase),
"Nested worktree path should NOT be under the centralized worktrees dir");
}

[Fact]
public void CreateWorktree_WithoutLocalPath_PlacesWorktreeInCentralDir()
{
// When localPath is null, the worktree path should be:
// {WorktreesDir}/{repoId}-{guid8}
// This is the "centralized strategy" for URL-based groups.
// All worktrees should go to {WorktreesDir}/{repoId}-{guid8}
// (centralized strategy — nested strategy was removed).

var testBaseDir = Path.Combine(Path.GetTempPath(), $"central-strategy-{Guid.NewGuid():N}");
var worktreesDir = Path.Combine(testBaseDir, "worktrees");
Expand All @@ -1037,24 +1012,70 @@ public void CreateWorktree_WithoutLocalPath_PlacesWorktreeInCentralDir()
Assert.True(expectedPath.StartsWith(worktreesDir, StringComparison.OrdinalIgnoreCase),
$"Centralized path '{expectedPath}' should be under WorktreesDir '{worktreesDir}'");

// Verify it does NOT contain .polypilot/worktrees (which would indicate nested)
// Verify it does NOT contain .polypilot/worktrees (which would indicate old nested strategy)
var marker = Path.Combine(".polypilot", "worktrees");
Assert.DoesNotContain(marker, expectedPath, StringComparison.OrdinalIgnoreCase);
}

#endregion

#region Existing Folder Safety Tests

[Fact]
public void RemoveRepository_DeleteFromDisk_SkipsNonManagedBareClonePath()
{
// Regression: repos added via "Existing Folder" have BareClonePath pointing
// at the user's real project directory. RemoveRepositoryAsync with deleteFromDisk
// must NOT delete it — only managed bare clones under ReposDir should be deleted.

var testDir = Path.Combine(Path.GetTempPath(), $"polypilot-tests-{Guid.NewGuid():N}");
var userProject = Path.Combine(testDir, "user-project");
var reposDir = Path.Combine(testDir, "repos");
Directory.CreateDirectory(userProject);
File.WriteAllText(Path.Combine(userProject, "important.txt"), "don't delete me");
Directory.CreateDirectory(reposDir);

// Verify the user's project path does NOT start with the managed repos dir
var fullUserProject = Path.GetFullPath(userProject);
var managedPrefix = Path.GetFullPath(reposDir) + Path.DirectorySeparatorChar;
Assert.False(fullUserProject.StartsWith(managedPrefix, StringComparison.OrdinalIgnoreCase),
"Test setup error: user project should not be under the managed repos dir");

// Verify that user's project still exists (the guard should prevent deletion)
Assert.True(Directory.Exists(userProject));
Assert.True(File.Exists(Path.Combine(userProject, "important.txt")));

// Clean up
try { Directory.Delete(testDir, recursive: true); } catch { }
}

[Fact]
public void CreateWorktree_LocalPath_StrategySelectedByNullCheck()
public void WorktreeReuse_OnlyMatchesCentralizedWorktrees()
{
// Regression: the localPath parameter is the SOLE discriminator between nested
// and centralized strategy. Verify that an empty/whitespace localPath would NOT
// accidentally trigger the nested path (same guard that CreateWorktreeAsync uses).

// Production code: if (!string.IsNullOrWhiteSpace(localPath)) → nested
Assert.True(string.IsNullOrWhiteSpace(null));
Assert.True(string.IsNullOrWhiteSpace(""));
Assert.True(string.IsNullOrWhiteSpace(" "));
Assert.False(string.IsNullOrWhiteSpace("/valid/path"));
Assert.False(string.IsNullOrWhiteSpace(@"C:\valid\path"));
// Regression: worktree reuse must only return worktrees under the centralized
// WorktreesDir, not external user checkouts registered via "Existing Folder".

var testDir = Path.Combine(Path.GetTempPath(), $"polypilot-tests-{Guid.NewGuid():N}");
var worktreesDir = Path.Combine(testDir, "worktrees");
var userCheckout = Path.Combine(testDir, "user-project");
Directory.CreateDirectory(worktreesDir);
Directory.CreateDirectory(userCheckout);

// External worktree path should NOT start with the centralized WorktreesDir
var fullUserPath = Path.GetFullPath(userCheckout);
var managedPrefix = Path.GetFullPath(worktreesDir) + Path.DirectorySeparatorChar;
Assert.False(fullUserPath.StartsWith(managedPrefix, StringComparison.OrdinalIgnoreCase),
"External user checkout should NOT be matched by the centralized-only worktree reuse logic");

// A managed worktree SHOULD match
var managedWorktree = Path.Combine(worktreesDir, "repo-abc12345");
Directory.CreateDirectory(managedWorktree);
var fullManagedPath = Path.GetFullPath(managedWorktree);
Assert.True(fullManagedPath.StartsWith(managedPrefix, StringComparison.OrdinalIgnoreCase),
"Managed worktree should be under the centralized WorktreesDir");

// Clean up
try { Directory.Delete(testDir, recursive: true); } catch { }
}

#endregion
Expand Down
4 changes: 2 additions & 2 deletions PolyPilot.Tests/WorktreeStrategyTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public FakeRepoManager(List<RepositoryInfo> repos)
}

public override Task<WorktreeInfo> CreateWorktreeAsync(string repoId, string branchName,
string? baseBranch = null, bool skipFetch = false, string? localPath = null, CancellationToken ct = default)
string? baseBranch = null, bool skipFetch = false, CancellationToken ct = default)
{
CreateCalls.Add((repoId, branchName, skipFetch));
var id = $"wt-{Interlocked.Increment(ref _worktreeCounter)}";
Expand Down Expand Up @@ -560,7 +560,7 @@ public FailingRepoManager(List<RepositoryInfo> repos)
}

public override Task<WorktreeInfo> CreateWorktreeAsync(string repoId, string branchName,
string? baseBranch = null, bool skipFetch = false, string? localPath = null, CancellationToken ct = default)
string? baseBranch = null, bool skipFetch = false, CancellationToken ct = default)
{
throw new InvalidOperationException("Simulated git failure");
}
Expand Down
Loading