Address PR review: safety guards, scoped reuse, orphan cleanup

- Guard RemoveRepositoryAsync to only delete BareClonePath under managed
  ReposDir, preventing recursive deletion of user's real project
- Restrict worktree reuse to centralized WorktreesDir only — external
  user checkouts are never returned to avoid multi-session conflicts
- Clean up orphaned managed bare clone when same repo is re-added via
  Existing Folder (prevents disk waste)
- Fix GetDefaultBranch to prefer origin/HEAD over symbolic-ref HEAD so
  non-bare repos don't branch from the wrong base
- Revert testhost from production FindActiveLockPid (test-only concern)
- Update RunGhAsync comment to reflect non-bare repo support
- Add regression tests for delete guard and worktree reuse scoping

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: PureWeen

PolyPilot.Tests/ExternalSessionScannerTests.cs

@@ -502,8 +502,7 @@ public void FindActiveLockPid_DetectsCurrentProcess()
 
             var myName = System.Diagnostics.Process.GetCurrentProcess().ProcessName.ToLowerInvariant();
             var matchesFilter = myName.Contains("copilot") || myName.Contains("node") ||
-                                myName.Contains("dotnet") || myName.Contains("github") ||
-                                myName.Contains("testhost");
+                                myName.Contains("dotnet") || myName.Contains("github");
 
             var scanner = new ExternalSessionScanner(_sessionStateDir, () => new HashSet<string>());
             var detectedPid = scanner.FindActiveLockPid(testDir);

PolyPilot.Tests/RepoManagerTests.cs

@@ -1019,6 +1019,67 @@ public void CreateWorktree_AlwaysPlacesWorktreeInCentralDir()
 
     #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 WorktreeReuse_OnlyMatchesCentralizedWorktrees()
+    {
+        // 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
+
     #region M2 Migration Ambiguity Tests
 
     [Fact]

PolyPilot/Services/ExternalSessionScanner.cs

@@ -505,8 +505,7 @@ private static bool ComputeNeedsAttention(List<ChatMessage> history)
                         // plausibly belongs to a Copilot CLI or its host runtime.
                         var name = proc.ProcessName?.ToLowerInvariant() ?? "";
                         if (!name.Contains("copilot") && !name.Contains("node") &&
-                            !name.Contains("dotnet") && !name.Contains("github") &&
-                            !name.Contains("testhost"))
+                            !name.Contains("dotnet") && !name.Contains("github"))
                             continue;
                         return pid;
                     }

PolyPilot/Services/RepoManager.cs

@@ -589,11 +589,21 @@ public async Task<RepositoryInfo> AddRepositoryFromLocalAsync(
         var id = RepoIdFromUrl(url);
 
         RepositoryInfo repo;
+        string? oldBareClonePath = null;
         lock (_stateLock)
         {
             var existing = _state.Repositories.FirstOrDefault(r => r.Id == id);
             if (existing != null)
             {
+                // If the old BareClonePath was a managed bare clone, remember it for cleanup.
+                if (!string.IsNullOrWhiteSpace(existing.BareClonePath)
+                    && !PathsEqual(existing.BareClonePath, localPath))
+                {
+                    var fullOld = Path.GetFullPath(existing.BareClonePath);
+                    var managedPrefix = Path.GetFullPath(ReposDir) + Path.DirectorySeparatorChar;
+                    if (fullOld.StartsWith(managedPrefix, StringComparison.OrdinalIgnoreCase))
+                        oldBareClonePath = existing.BareClonePath;
+                }
                 existing.BareClonePath = localPath;
                 BackfillWorktreeClonePaths(existing);
                 repo = existing;
@@ -614,6 +624,13 @@ public async Task<RepositoryInfo> AddRepositoryFromLocalAsync(
         Save();
         OnStateChanged?.Invoke();
 
+        // Clean up orphaned managed bare clone (if any) after state is saved.
+        if (oldBareClonePath != null && Directory.Exists(oldBareClonePath))
+        {
+            try { Directory.Delete(oldBareClonePath, recursive: true); }
+            catch (Exception ex) { Console.WriteLine($"[RepoManager] Failed to clean up old bare clone at '{oldBareClonePath}': {ex.Message}"); }
+        }
+
         // Register the local folder as an external worktree so it also appears in the
         // "📂 Existing" picker when creating repo-based sessions.
         await RegisterExternalWorktreeAsync(repo, localPath, ct);
@@ -706,16 +723,18 @@ public virtual async Task<WorktreeInfo> CreateWorktreeAsync(string repoId, strin
         var repo = _state.Repositories.FirstOrDefault(r => r.Id == repoId)
             ?? throw new InvalidOperationException($"Repository '{repoId}' not found.");
 
-        // Check if an existing registered worktree for this repo is already on the requested branch.
-        // This handles the common case where the user added their repo via "Existing Folder" and
-        // wants to create a session on the same branch — no need to create a duplicate worktree.
+        // Check if an existing PolyPilot-managed worktree for this repo is already on the requested branch.
+        // Only reuse worktrees under the centralized WorktreesDir — never return the user's own
+        // checkout (registered as an external worktree) to avoid multiple sessions sharing it.
         WorktreeInfo? existingMatch;
+        var managedWorktreePrefix = Path.GetFullPath(WorktreesDir) + Path.DirectorySeparatorChar;
         lock (_stateLock)
         {
             existingMatch = _state.Worktrees.FirstOrDefault(w =>
                 w.RepoId == repoId
                 && string.Equals(w.Branch, branchName, StringComparison.OrdinalIgnoreCase)
                 && !string.IsNullOrWhiteSpace(w.Path)
+                && Path.GetFullPath(w.Path).StartsWith(managedWorktreePrefix, StringComparison.OrdinalIgnoreCase)
                 && Directory.Exists(w.Path));
         }
         if (existingMatch != null)
@@ -1091,7 +1110,15 @@ public async Task RemoveRepositoryAsync(string repoId, bool deleteFromDisk, Canc
 
         if (deleteFromDisk && Directory.Exists(repo.BareClonePath))
         {
-            try { Directory.Delete(repo.BareClonePath, recursive: true); } catch { }
+            // Only delete if BareClonePath is under the managed ReposDir.
+            // Repos added via "Existing Folder" have BareClonePath pointing at the user's
+            // real project directory — we must NEVER delete that.
+            var fullClonePath = Path.GetFullPath(repo.BareClonePath);
+            var managedPrefix = Path.GetFullPath(ReposDir) + Path.DirectorySeparatorChar;
+            if (fullClonePath.StartsWith(managedPrefix, StringComparison.OrdinalIgnoreCase))
+            {
+                try { Directory.Delete(repo.BareClonePath, recursive: true); } catch { }
+            }
         }
 
         OnStateChanged?.Invoke();
@@ -1154,7 +1181,20 @@ private async Task<string> GetDefaultBranch(string barePath, CancellationToken c
     {
         try
         {
-            // Get the default branch name (e.g. "main")
+            // Prefer origin/HEAD which points at the canonical default branch regardless
+            // of which branch is currently checked out (important for non-bare repos).
+            try
+            {
+                var originHead = (await RunGitAsync(barePath, ct, "rev-parse", "--abbrev-ref", "origin/HEAD")).Trim();
+                if (!string.IsNullOrWhiteSpace(originHead) && originHead != "origin/HEAD")
+                {
+                    Console.WriteLine($"[RepoManager] Using origin/HEAD: {originHead}");
+                    return $"refs/remotes/{originHead}";
+                }
+            }
+            catch { /* origin/HEAD not set — fall through */ }
+
+            // Fallback: use symbolic-ref HEAD (correct for bare repos, may be wrong for non-bare)
             var headRef = await RunGitAsync(barePath, ct, "symbolic-ref", "HEAD");
             var branchName = headRef.Trim().Replace("refs/heads/", "");
 
@@ -1208,7 +1248,9 @@ private static async Task<string> RunGhAsync(string? workDir, CancellationToken
         if (workDir != null)
         {
             psi.WorkingDirectory = workDir;
-            // Bare repos need GIT_DIR set explicitly for gh to find the remote
+            // Bare repos (paths ending in .git) need GIT_DIR set explicitly for gh
+            // to find the remote. Non-bare repos (including those added via "Existing Folder")
+            // don't need this — gh discovers the remote from the working directory.
             if (workDir.EndsWith(".git", StringComparison.OrdinalIgnoreCase))
                 psi.Environment["GIT_DIR"] = workDir;
         }