From be501d0c79112cab8a080f1a1c504490a7286055 Mon Sep 17 00:00:00 2001 From: Shane Date: Tue, 24 Feb 2026 13:29:20 -0600 Subject: [PATCH 01/33] Streamline session+worktree creation: atomic API, quick-create, inline branch input - Add CreateSessionWithWorktreeAsync() to CopilotService: single atomic call that creates worktree, creates session, links them, organizes into repo group, and optionally sends initial prompt. Includes rollback on session creation failure. - Add WorktreeId property to AgentSessionInfo for first-class session-to-worktree relationship (previously only tracked via path string and SessionMeta). - Add Quick Session button (lightning bolt) to repo group context menu: one-click to auto-generate branch, create worktree+session, and switch to it. - Add New Branch + Session button to repo group context menu: opens inline input below group header where user types branch name (or #PR number) and presses Enter to atomically create worktree+session. Replaces the 7-step form flow. - Refactor HandleCreateSession to use atomic API when worktreeId is provided, eliminating scattered linking/organizing logic. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Components/Layout/SessionSidebar.razor | 154 +++++++++++++++--- .../Layout/SessionSidebar.razor.css | 55 +++++++ PolyPilot/Models/AgentSessionInfo.cs | 2 + PolyPilot/Services/CopilotService.cs | 80 +++++++++ 4 files changed, 270 insertions(+), 21 deletions(-) diff --git a/PolyPilot/Components/Layout/SessionSidebar.razor b/PolyPilot/Components/Layout/SessionSidebar.razor index 5707392198..8f2dca762b 100644 --- a/PolyPilot/Components/Layout/SessionSidebar.razor +++ b/PolyPilot/Components/Layout/SessionSidebar.razor @@ -441,6 +441,12 @@ else + + @@ -490,6 +496,32 @@ else } + @if (quickBranchRepoId == group.RepoId && isRepoGroup) + { + var qbRepoId = group.RepoId!; +
+ + + @if (quickBranchIsCreating) + { + + } + else + { + + + } + @if (!string.IsNullOrEmpty(quickBranchError)) + { +
⚠ @quickBranchError
+ } +
+ } + @if (group.IsMultiAgent && !group.IsCollapsed) { var maGroupId = group.Id; @@ -685,6 +717,12 @@ else private string? confirmRepoName = null; private string? confirmRemoveRepoId = null; + // Quick-create inline branch input + private string? quickBranchRepoId = null; + private string quickBranchInput = ""; + private bool quickBranchIsCreating = false; + private string? quickBranchError = null; + private async Task ToggleFlyout() { await OnToggleFlyout.InvokeAsync(); @@ -890,36 +928,33 @@ else createError = null; try { - var sessionInfo = await CopilotService.CreateSessionAsync(args.Name, args.Model, args.Directory); - CopilotService.SwitchSession(sessionInfo.Name); - newSessionName = ""; + AgentSessionInfo sessionInfo; - // If created from a worktree, place session in the repo group if (!string.IsNullOrEmpty(args.WorktreeId)) { - var wt = RepoManager.Worktrees.FirstOrDefault(w => w.Id == args.WorktreeId); - if (wt != null) + // Atomic worktree+session creation + sessionInfo = await CopilotService.CreateSessionWithWorktreeAsync( + repoId: null!, // Not needed when worktreeId is provided + worktreeId: args.WorktreeId, + sessionName: args.Name, + model: args.Model, + initialPrompt: args.InitialPrompt); + } + else + { + sessionInfo = await CopilotService.CreateSessionAsync(args.Name, args.Model, args.Directory); + CopilotService.SwitchSession(sessionInfo.Name); + + // Send initial prompt after session is ready + if (!string.IsNullOrEmpty(args.InitialPrompt)) { - RepoManager.LinkSessionToWorktree(wt.Id, sessionInfo.Name); - var repo = RepoManager.Repositories.FirstOrDefault(r => r.Id == wt.RepoId); - if (repo != null) - { - var group = CopilotService.GetOrCreateRepoGroup(repo.Id, repo.Name); - CopilotService.MoveSession(sessionInfo.Name, group.Id); - var meta = CopilotService.GetSessionMeta(sessionInfo.Name); - if (meta != null) meta.WorktreeId = wt.Id; - } + _ = CopilotService.SendPromptAsync(sessionInfo.Name, args.InitialPrompt); } } + newSessionName = ""; CopilotService.SaveUiState(currentPage, selectedModel: selectedModel); await OnSessionSelected.InvokeAsync(); - - // Send initial prompt after session is ready - if (!string.IsNullOrEmpty(args.InitialPrompt)) - { - _ = CopilotService.SendPromptAsync(sessionInfo.Name, args.InitialPrompt); - } } catch (Exception ex) { @@ -932,6 +967,83 @@ else } } + private async Task QuickCreateSessionForRepo(string repoId) + { + if (isCreating) return; + isCreating = true; + createError = null; + try + { + var sessionInfo = await CopilotService.CreateSessionWithWorktreeAsync( + repoId: repoId, + model: selectedModel); + CopilotService.SaveUiState(currentPage, selectedModel: selectedModel); + await OnSessionSelected.InvokeAsync(); + } + catch (Exception ex) + { + createError = ex.Message; + Console.WriteLine($"Error quick-creating session: {ex}"); + } + finally + { + isCreating = false; + } + } + + private void StartQuickBranch(string repoId) + { + quickBranchRepoId = repoId; + quickBranchInput = ""; + quickBranchError = null; + } + + private async Task HandleQuickBranchKeyDown(KeyboardEventArgs e, string repoId) + { + if (e.Key == "Enter") await CommitQuickBranch(repoId); + else if (e.Key == "Escape") quickBranchRepoId = null; + } + + private async Task CommitQuickBranch(string repoId) + { + var input = quickBranchInput?.Trim(); + if (string.IsNullOrEmpty(input) || quickBranchIsCreating) return; + + quickBranchIsCreating = true; + quickBranchError = null; + StateHasChanged(); + try + { + int? prNumber = null; + string? branchName = null; + + if (input.StartsWith("#") && int.TryParse(input[1..], out var pr)) + prNumber = pr; + else + branchName = input; + + await CopilotService.CreateSessionWithWorktreeAsync( + repoId: repoId, + branchName: branchName, + prNumber: prNumber, + model: selectedModel); + + quickBranchRepoId = null; + quickBranchInput = ""; + CopilotService.SaveUiState(currentPage, selectedModel: selectedModel); + await OnSessionSelected.InvokeAsync(); + } + catch (Exception ex) + { + quickBranchError = ex.Message; + Console.WriteLine($"Error creating branch+session: {ex}"); + } + finally + { + quickBranchIsCreating = false; + } + } + private void ConfirmResume(PersistedSessionInfo persisted) { resumeError = null; diff --git a/PolyPilot/Components/Layout/SessionSidebar.razor.css b/PolyPilot/Components/Layout/SessionSidebar.razor.css index 175abc8bf2..ddd9730f17 100644 --- a/PolyPilot/Components/Layout/SessionSidebar.razor.css +++ b/PolyPilot/Components/Layout/SessionSidebar.razor.css @@ -1634,3 +1634,58 @@ .adjust-banner span { line-height: 1.3; } + +/* Quick branch input bar */ +.quick-branch-input-bar { + display: flex; + align-items: center; + gap: 4px; + padding: 4px 8px; + background: var(--panel-bg); + border-bottom: 1px solid var(--control-border); + flex-wrap: wrap; +} +.quick-branch-icon { + font-size: 0.85rem; + color: var(--text-secondary); + flex-shrink: 0; +} +.quick-branch-input { + flex: 1; + min-width: 80px; + padding: 3px 6px; + font-size: 0.78rem; + border: 1px solid var(--control-border); + border-radius: 4px; + background: var(--panel-bg); + color: var(--text-primary); + outline: none; +} +.quick-branch-input:focus { + border-color: var(--accent-secondary, #58a6ff); +} +.quick-branch-go-btn, +.quick-branch-cancel-btn { + padding: 2px 6px; + border: none; + border-radius: 3px; + cursor: pointer; + font-size: 0.78rem; + background: var(--control-border); + color: var(--text-primary); + flex-shrink: 0; +} +.quick-branch-go-btn:hover { background: var(--accent-secondary, #58a6ff); color: #fff; } +.quick-branch-cancel-btn:hover { background: var(--accent-primary); color: #fff; } +.quick-branch-go-btn:disabled { opacity: 0.4; cursor: default; } +.quick-branch-spinner { + font-size: 0.85rem; + animation: spin 1s linear infinite; +} +.quick-branch-error { + width: 100%; + font-size: 0.72rem; + color: var(--accent-primary); + padding: 2px 0 0; +} +@keyframes spin { to { transform: rotate(360deg); } } diff --git a/PolyPilot/Models/AgentSessionInfo.cs b/PolyPilot/Models/AgentSessionInfo.cs index b931766282..59a3942168 100644 --- a/PolyPilot/Models/AgentSessionInfo.cs +++ b/PolyPilot/Models/AgentSessionInfo.cs @@ -12,6 +12,8 @@ public class AgentSessionInfo public string? WorkingDirectory { get; set; } public string? GitBranch { get; set; } + /// Worktree ID if this session was created from a worktree. + public string? WorktreeId { get; set; } // For resumed sessions public string? SessionId { get; set; } diff --git a/PolyPilot/Services/CopilotService.cs b/PolyPilot/Services/CopilotService.cs index 74989abad3..fa3fae857e 100644 --- a/PolyPilot/Services/CopilotService.cs +++ b/PolyPilot/Services/CopilotService.cs @@ -1401,6 +1401,86 @@ ALWAYS run the relaunch script as the final step after making changes to this pr return info; } + /// + /// Atomically creates a worktree (or uses an existing one) and a session linked to it. + /// Handles worktree creation, session creation, linking, group organization, and optional initial prompt. + /// + public async Task CreateSessionWithWorktreeAsync( + string repoId, + string? branchName = null, + int? prNumber = null, + string? worktreeId = null, + string? sessionName = null, + string? model = null, + string? initialPrompt = null, + CancellationToken ct = default) + { + WorktreeInfo wt; + + if (!string.IsNullOrEmpty(worktreeId)) + { + // Use existing worktree + wt = _repoManager.Worktrees.FirstOrDefault(w => w.Id == worktreeId) + ?? throw new InvalidOperationException($"Worktree '{worktreeId}' not found."); + } + else if (prNumber.HasValue) + { + wt = await _repoManager.CreateWorktreeFromPrAsync(repoId, prNumber.Value, ct); + } + else + { + var branch = branchName ?? $"session-{DateTime.Now:yyyyMMdd-HHmmss}"; + wt = await _repoManager.CreateWorktreeAsync(repoId, branch, null, ct); + } + + var name = sessionName ?? wt.Branch; + + // Ensure unique session name + if (_sessions.ContainsKey(name)) + { + var suffix = DateTime.Now.ToString("HHmm"); + name = $"{name}-{suffix}"; + } + + AgentSessionInfo sessionInfo; + try + { + sessionInfo = await CreateSessionAsync(name, model, wt.Path, ct); + } + catch + { + // If session creation fails and we just created a new worktree, clean up + if (string.IsNullOrEmpty(worktreeId)) + { + try { await _repoManager.RemoveWorktreeAsync(wt.Id); } catch { } + } + throw; + } + + // Link session to worktree + sessionInfo.WorktreeId = wt.Id; + _repoManager.LinkSessionToWorktree(wt.Id, sessionInfo.Name); + + // Organize into repo group + var repo = _repoManager.Repositories.FirstOrDefault(r => r.Id == wt.RepoId); + if (repo != null) + { + var group = GetOrCreateRepoGroup(repo.Id, repo.Name); + MoveSession(sessionInfo.Name, group.Id); + var meta = GetSessionMeta(sessionInfo.Name); + if (meta != null) meta.WorktreeId = wt.Id; + } + + SwitchSession(sessionInfo.Name); + SaveActiveSessionsToDisk(); + + // Send initial prompt after session is ready + if (!string.IsNullOrEmpty(initialPrompt)) + _ = SendPromptAsync(sessionInfo.Name, initialPrompt); + + return sessionInfo; + } + /// /// Destroys the existing session and creates a new one with the same name but a different model. /// Use this for "changing" the model of an empty session. From 35811c61ba8d624b6c1206c2fcbfcd81abb936a6 Mon Sep 17 00:00:00 2001 From: Shane Date: Tue, 24 Feb 2026 14:23:49 -0600 Subject: [PATCH 02/33] Rename menu items: Quick Branch + Session / Named Branch + Session MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clarifies that both options create a branch — the difference is whether you name it yourself or get an auto-generated name. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- PolyPilot/Components/Layout/SessionSidebar.razor | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/PolyPilot/Components/Layout/SessionSidebar.razor b/PolyPilot/Components/Layout/SessionSidebar.razor index 8f2dca762b..abcf78b701 100644 --- a/PolyPilot/Components/Layout/SessionSidebar.razor +++ b/PolyPilot/Components/Layout/SessionSidebar.razor @@ -442,10 +442,10 @@ else ➕ New Session + } + + } @{ var currentGroup = Groups?.FirstOrDefault(g => g.Id == Meta?.GroupId); } @@ -227,6 +247,7 @@ [Parameter] public EventCallback OnCloseAndDeleteWorktree { get; set; } [Parameter] public EventCallback OnPin { get; set; } [Parameter] public EventCallback OnMove { get; set; } + [Parameter] public EventCallback OnMoveToWorktree { get; set; } [Parameter] public EventCallback OnStartRename { get; set; } [Parameter] public EventCallback OnCommitRename { get; set; } [Parameter] public EventCallback OnToggleMenu { get; set; } diff --git a/PolyPilot/Components/Layout/SessionSidebar.razor b/PolyPilot/Components/Layout/SessionSidebar.razor index abcf78b701..5a4a9acccc 100644 --- a/PolyPilot/Components/Layout/SessionSidebar.razor +++ b/PolyPilot/Components/Layout/SessionSidebar.razor @@ -615,6 +615,7 @@ else OnCloseAndDeleteWorktree="() => CloseSessionAndDeleteWorktree(sName)" OnPin="(pinned) => { CopilotService.PinSession(sName, pinned); }" OnMove="(groupId) => CopilotService.MoveSession(sName, groupId)" + OnMoveToWorktree="(wtId) => CopilotService.MoveSessionToWorktree(sName, wtId)" OnStartRename="() => StartRename(sName)" OnCommitRename="CommitRename" OnToggleMenu="() => ToggleSessionMenu(sName)" diff --git a/PolyPilot/Services/CopilotService.cs b/PolyPilot/Services/CopilotService.cs index 01ab421272..8fae58e5a6 100644 --- a/PolyPilot/Services/CopilotService.cs +++ b/PolyPilot/Services/CopilotService.cs @@ -1488,6 +1488,41 @@ public async Task CreateSessionWithWorktreeAsync( return sessionInfo; } + /// + /// Moves a session to a different worktree, updating its WorkingDirectory, + /// WorktreeId, git branch, repo group membership, and worktree link. + /// + public void MoveSessionToWorktree(string sessionName, string worktreeId) + { + if (!_sessions.TryGetValue(sessionName, out var state)) return; + + var wt = _repoManager.Worktrees.FirstOrDefault(w => w.Id == worktreeId); + if (wt == null) return; + + // Update session info + state.Info.WorkingDirectory = wt.Path; + state.Info.WorktreeId = wt.Id; + state.Info.GitBranch = GetGitBranch(wt.Path); + + // Update worktree link + _repoManager.LinkSessionToWorktree(wt.Id, sessionName); + + // Update session meta + var meta = GetSessionMeta(sessionName); + if (meta != null) meta.WorktreeId = wt.Id; + + // Move to the worktree's repo group + var repo = _repoManager.Repositories.FirstOrDefault(r => r.Id == wt.RepoId); + if (repo != null) + { + var group = GetOrCreateRepoGroup(repo.Id, repo.Name); + MoveSession(sessionName, group.Id); + } + + SaveActiveSessionsToDisk(); + OnStateChanged?.Invoke(); + } + /// /// Destroys the existing session and creates a new one with the same name but a different model. /// Use this for "changing" the model of an empty session. From b121516cc9f3cc3acfc274f770c8f1dba82a8824 Mon Sep 17 00:00:00 2001 From: Shane Date: Tue, 24 Feb 2026 15:31:56 -0600 Subject: [PATCH 06/33] Rework 'Move to Worktree' to create new worktree and preserve session Replaces the previous 'list existing worktrees' submenu with a single 'Move to New Worktree...' menu item that opens an inline input below the session. User types a branch name (or #PR) and the session is moved to the new worktree while preserving the Copilot session and conversation history. - MoveSessionToNewWorktreeAsync() creates a new worktree, updates the session's WorkingDirectory/WorktreeId/GitBranch/meta/group, then sends a message to the agent notifying it of the directory change. - Inline input supports repo selector when multiple repos exist. - Session history and SDK session are fully preserved. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Components/Layout/SessionListItem.razor | 25 ++--- .../Components/Layout/SessionSidebar.razor | 95 ++++++++++++++++++- .../Layout/SessionSidebar.razor.css | 10 ++ PolyPilot/Services/CopilotService.cs | 30 +++++- 4 files changed, 135 insertions(+), 25 deletions(-) diff --git a/PolyPilot/Components/Layout/SessionListItem.razor b/PolyPilot/Components/Layout/SessionListItem.razor index ce113bc0e6..8d332c2f84 100644 --- a/PolyPilot/Components/Layout/SessionListItem.razor +++ b/PolyPilot/Components/Layout/SessionListItem.razor @@ -127,25 +127,12 @@ } } - @{ - var availableWorktrees = RepoManager.Worktrees - .Where(w => w.Id != Meta?.WorktreeId) - .ToList(); - } - @if (availableWorktrees.Count > 0) + @if (RepoManager.Repositories.Count > 0) { - + + } @{ var currentGroup = Groups?.FirstOrDefault(g => g.Id == Meta?.GroupId); @@ -247,7 +234,7 @@ [Parameter] public EventCallback OnCloseAndDeleteWorktree { get; set; } [Parameter] public EventCallback OnPin { get; set; } [Parameter] public EventCallback OnMove { get; set; } - [Parameter] public EventCallback OnMoveToWorktree { get; set; } + [Parameter] public EventCallback OnMoveToNewWorktree { get; set; } [Parameter] public EventCallback OnStartRename { get; set; } [Parameter] public EventCallback OnCommitRename { get; set; } [Parameter] public EventCallback OnToggleMenu { get; set; } diff --git a/PolyPilot/Components/Layout/SessionSidebar.razor b/PolyPilot/Components/Layout/SessionSidebar.razor index 5a4a9acccc..df40550ac7 100644 --- a/PolyPilot/Components/Layout/SessionSidebar.razor +++ b/PolyPilot/Components/Layout/SessionSidebar.razor @@ -615,13 +615,47 @@ else OnCloseAndDeleteWorktree="() => CloseSessionAndDeleteWorktree(sName)" OnPin="(pinned) => { CopilotService.PinSession(sName, pinned); }" OnMove="(groupId) => CopilotService.MoveSession(sName, groupId)" - OnMoveToWorktree="(wtId) => CopilotService.MoveSessionToWorktree(sName, wtId)" + OnMoveToNewWorktree="() => StartMoveToWorktree(sName)" OnStartRename="() => StartRename(sName)" OnCommitRename="CommitRename" OnToggleMenu="() => ToggleSessionMenu(sName)" OnCloseMenu="() => { openMenuSession = null; }" OnReportBug="() => OpenBugReportForSession(sName)" OnFixWithCopilot="() => OpenFixItForSession(sName)" /> + @if (moveToWorktreeSessionName == sName) + { + var mtRepos = RepoManager.Repositories.ToList(); +
+ + @if (mtRepos.Count > 1) + { + + } + + @if (moveToWorktreeIsCreating) + { + + } + else + { + + + } + @if (!string.IsNullOrEmpty(moveToWorktreeError)) + { +
⚠ @moveToWorktreeError
+ } +
+ } } } } @@ -724,6 +758,13 @@ else private bool quickBranchIsCreating = false; private string? quickBranchError = null; + // Move session to new worktree + private string? moveToWorktreeSessionName = null; + private string moveToWorktreeInput = ""; + private string? moveToWorktreeRepoId = null; + private bool moveToWorktreeIsCreating = false; + private string? moveToWorktreeError = null; + private async Task ToggleFlyout() { await OnToggleFlyout.InvokeAsync(); @@ -1045,6 +1086,58 @@ else } } + private void StartMoveToWorktree(string sessionName) + { + moveToWorktreeSessionName = sessionName; + moveToWorktreeInput = ""; + moveToWorktreeError = null; + moveToWorktreeRepoId = RepoManager.Repositories.FirstOrDefault()?.Id; + } + + private async Task HandleMoveToWorktreeKeyDown(KeyboardEventArgs e) + { + if (e.Key == "Enter") await CommitMoveToWorktree(); + else if (e.Key == "Escape") moveToWorktreeSessionName = null; + } + + private async Task CommitMoveToWorktree() + { + var input = moveToWorktreeInput?.Trim(); + var sessionName = moveToWorktreeSessionName; + var repoId = moveToWorktreeRepoId; + if (string.IsNullOrEmpty(input) || string.IsNullOrEmpty(sessionName) + || string.IsNullOrEmpty(repoId) || moveToWorktreeIsCreating) return; + + moveToWorktreeIsCreating = true; + moveToWorktreeError = null; + StateHasChanged(); + try + { + int? prNumber = null; + string? branchName = null; + + if (input.StartsWith("#") && int.TryParse(input[1..], out var pr)) + prNumber = pr; + else + branchName = input; + + await CopilotService.MoveSessionToNewWorktreeAsync( + sessionName, repoId, branchName, prNumber); + + moveToWorktreeSessionName = null; + moveToWorktreeInput = ""; + } + catch (Exception ex) + { + moveToWorktreeError = ex.Message; + Console.WriteLine($"Error moving session to worktree: {ex}"); + } + finally + { + moveToWorktreeIsCreating = false; + } + } + private void ConfirmResume(PersistedSessionInfo persisted) { resumeError = null; diff --git a/PolyPilot/Components/Layout/SessionSidebar.razor.css b/PolyPilot/Components/Layout/SessionSidebar.razor.css index 8f962aad39..f632bd5e07 100644 --- a/PolyPilot/Components/Layout/SessionSidebar.razor.css +++ b/PolyPilot/Components/Layout/SessionSidebar.razor.css @@ -1691,3 +1691,13 @@ padding: 2px 0 0; } @keyframes spin { to { transform: rotate(360deg); } } +.quick-branch-repo-select { + padding: 3px 4px; + font-size: 0.75rem; + border: 1px solid var(--control-border); + border-radius: 4px; + background: var(--panel-bg); + color: var(--text-primary); + max-width: 100px; + flex-shrink: 0; +} diff --git a/PolyPilot/Services/CopilotService.cs b/PolyPilot/Services/CopilotService.cs index 8fae58e5a6..c0d586a9bd 100644 --- a/PolyPilot/Services/CopilotService.cs +++ b/PolyPilot/Services/CopilotService.cs @@ -1489,15 +1489,29 @@ public async Task CreateSessionWithWorktreeAsync( } /// - /// Moves a session to a different worktree, updating its WorkingDirectory, - /// WorktreeId, git branch, repo group membership, and worktree link. + /// Creates a new worktree and moves the session to it, preserving the Copilot session + /// and conversation history. Notifies the agent about the directory change. /// - public void MoveSessionToWorktree(string sessionName, string worktreeId) + public async Task MoveSessionToNewWorktreeAsync( + string sessionName, + string repoId, + string? branchName = null, + int? prNumber = null, + CancellationToken ct = default) { if (!_sessions.TryGetValue(sessionName, out var state)) return; - var wt = _repoManager.Worktrees.FirstOrDefault(w => w.Id == worktreeId); - if (wt == null) return; + // Create the new worktree + WorktreeInfo wt; + if (prNumber.HasValue) + wt = await _repoManager.CreateWorktreeFromPrAsync(repoId, prNumber.Value, ct); + else + { + var branch = branchName ?? $"session-{DateTime.Now:yyyyMMdd-HHmmss}"; + wt = await _repoManager.CreateWorktreeAsync(repoId, branch, null, ct); + } + + var oldDir = state.Info.WorkingDirectory; // Update session info state.Info.WorkingDirectory = wt.Path; @@ -1521,6 +1535,12 @@ public void MoveSessionToWorktree(string sessionName, string worktreeId) SaveActiveSessionsToDisk(); OnStateChanged?.Invoke(); + + // Notify the agent about the directory change so it uses the new path + _ = SendPromptAsync(sessionName, + $"IMPORTANT: Your working directory has changed from `{oldDir}` to `{wt.Path}` (branch: `{wt.Branch}`). " + + $"All file operations should now use this new directory. Please acknowledge.", + skipHistoryMessage: false); } /// From 4f2147a80f937cee411613dab227f244b3f23ced Mon Sep 17 00:00:00 2001 From: Shane Date: Tue, 24 Feb 2026 15:37:31 -0600 Subject: [PATCH 07/33] Remove 'Move to Worktree' feature The SDK doesn't support changing a session's working directory at runtime, so moving a session to a different worktree can't reliably redirect the agent's file operations. Removed entirely rather than shipping a half-measure. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Components/Layout/SessionListItem.razor | 8 -- .../Components/Layout/SessionSidebar.razor | 94 ------------------- .../Layout/SessionSidebar.razor.css | 10 -- PolyPilot/Services/CopilotService.cs | 55 ----------- 4 files changed, 167 deletions(-) diff --git a/PolyPilot/Components/Layout/SessionListItem.razor b/PolyPilot/Components/Layout/SessionListItem.razor index 8d332c2f84..f032a86d42 100644 --- a/PolyPilot/Components/Layout/SessionListItem.razor +++ b/PolyPilot/Components/Layout/SessionListItem.razor @@ -127,13 +127,6 @@ } } - @if (RepoManager.Repositories.Count > 0) - { - - - } @{ var currentGroup = Groups?.FirstOrDefault(g => g.Id == Meta?.GroupId); } @@ -234,7 +227,6 @@ [Parameter] public EventCallback OnCloseAndDeleteWorktree { get; set; } [Parameter] public EventCallback OnPin { get; set; } [Parameter] public EventCallback OnMove { get; set; } - [Parameter] public EventCallback OnMoveToNewWorktree { get; set; } [Parameter] public EventCallback OnStartRename { get; set; } [Parameter] public EventCallback OnCommitRename { get; set; } [Parameter] public EventCallback OnToggleMenu { get; set; } diff --git a/PolyPilot/Components/Layout/SessionSidebar.razor b/PolyPilot/Components/Layout/SessionSidebar.razor index df40550ac7..abcf78b701 100644 --- a/PolyPilot/Components/Layout/SessionSidebar.razor +++ b/PolyPilot/Components/Layout/SessionSidebar.razor @@ -615,47 +615,12 @@ else OnCloseAndDeleteWorktree="() => CloseSessionAndDeleteWorktree(sName)" OnPin="(pinned) => { CopilotService.PinSession(sName, pinned); }" OnMove="(groupId) => CopilotService.MoveSession(sName, groupId)" - OnMoveToNewWorktree="() => StartMoveToWorktree(sName)" OnStartRename="() => StartRename(sName)" OnCommitRename="CommitRename" OnToggleMenu="() => ToggleSessionMenu(sName)" OnCloseMenu="() => { openMenuSession = null; }" OnReportBug="() => OpenBugReportForSession(sName)" OnFixWithCopilot="() => OpenFixItForSession(sName)" /> - @if (moveToWorktreeSessionName == sName) - { - var mtRepos = RepoManager.Repositories.ToList(); -
- - @if (mtRepos.Count > 1) - { - - } - - @if (moveToWorktreeIsCreating) - { - - } - else - { - - - } - @if (!string.IsNullOrEmpty(moveToWorktreeError)) - { -
⚠ @moveToWorktreeError
- } -
- } } } } @@ -758,13 +723,6 @@ else private bool quickBranchIsCreating = false; private string? quickBranchError = null; - // Move session to new worktree - private string? moveToWorktreeSessionName = null; - private string moveToWorktreeInput = ""; - private string? moveToWorktreeRepoId = null; - private bool moveToWorktreeIsCreating = false; - private string? moveToWorktreeError = null; - private async Task ToggleFlyout() { await OnToggleFlyout.InvokeAsync(); @@ -1086,58 +1044,6 @@ else } } - private void StartMoveToWorktree(string sessionName) - { - moveToWorktreeSessionName = sessionName; - moveToWorktreeInput = ""; - moveToWorktreeError = null; - moveToWorktreeRepoId = RepoManager.Repositories.FirstOrDefault()?.Id; - } - - private async Task HandleMoveToWorktreeKeyDown(KeyboardEventArgs e) - { - if (e.Key == "Enter") await CommitMoveToWorktree(); - else if (e.Key == "Escape") moveToWorktreeSessionName = null; - } - - private async Task CommitMoveToWorktree() - { - var input = moveToWorktreeInput?.Trim(); - var sessionName = moveToWorktreeSessionName; - var repoId = moveToWorktreeRepoId; - if (string.IsNullOrEmpty(input) || string.IsNullOrEmpty(sessionName) - || string.IsNullOrEmpty(repoId) || moveToWorktreeIsCreating) return; - - moveToWorktreeIsCreating = true; - moveToWorktreeError = null; - StateHasChanged(); - try - { - int? prNumber = null; - string? branchName = null; - - if (input.StartsWith("#") && int.TryParse(input[1..], out var pr)) - prNumber = pr; - else - branchName = input; - - await CopilotService.MoveSessionToNewWorktreeAsync( - sessionName, repoId, branchName, prNumber); - - moveToWorktreeSessionName = null; - moveToWorktreeInput = ""; - } - catch (Exception ex) - { - moveToWorktreeError = ex.Message; - Console.WriteLine($"Error moving session to worktree: {ex}"); - } - finally - { - moveToWorktreeIsCreating = false; - } - } - private void ConfirmResume(PersistedSessionInfo persisted) { resumeError = null; diff --git a/PolyPilot/Components/Layout/SessionSidebar.razor.css b/PolyPilot/Components/Layout/SessionSidebar.razor.css index f632bd5e07..8f962aad39 100644 --- a/PolyPilot/Components/Layout/SessionSidebar.razor.css +++ b/PolyPilot/Components/Layout/SessionSidebar.razor.css @@ -1691,13 +1691,3 @@ padding: 2px 0 0; } @keyframes spin { to { transform: rotate(360deg); } } -.quick-branch-repo-select { - padding: 3px 4px; - font-size: 0.75rem; - border: 1px solid var(--control-border); - border-radius: 4px; - background: var(--panel-bg); - color: var(--text-primary); - max-width: 100px; - flex-shrink: 0; -} diff --git a/PolyPilot/Services/CopilotService.cs b/PolyPilot/Services/CopilotService.cs index c0d586a9bd..01ab421272 100644 --- a/PolyPilot/Services/CopilotService.cs +++ b/PolyPilot/Services/CopilotService.cs @@ -1488,61 +1488,6 @@ public async Task CreateSessionWithWorktreeAsync( return sessionInfo; } - /// - /// Creates a new worktree and moves the session to it, preserving the Copilot session - /// and conversation history. Notifies the agent about the directory change. - /// - public async Task MoveSessionToNewWorktreeAsync( - string sessionName, - string repoId, - string? branchName = null, - int? prNumber = null, - CancellationToken ct = default) - { - if (!_sessions.TryGetValue(sessionName, out var state)) return; - - // Create the new worktree - WorktreeInfo wt; - if (prNumber.HasValue) - wt = await _repoManager.CreateWorktreeFromPrAsync(repoId, prNumber.Value, ct); - else - { - var branch = branchName ?? $"session-{DateTime.Now:yyyyMMdd-HHmmss}"; - wt = await _repoManager.CreateWorktreeAsync(repoId, branch, null, ct); - } - - var oldDir = state.Info.WorkingDirectory; - - // Update session info - state.Info.WorkingDirectory = wt.Path; - state.Info.WorktreeId = wt.Id; - state.Info.GitBranch = GetGitBranch(wt.Path); - - // Update worktree link - _repoManager.LinkSessionToWorktree(wt.Id, sessionName); - - // Update session meta - var meta = GetSessionMeta(sessionName); - if (meta != null) meta.WorktreeId = wt.Id; - - // Move to the worktree's repo group - var repo = _repoManager.Repositories.FirstOrDefault(r => r.Id == wt.RepoId); - if (repo != null) - { - var group = GetOrCreateRepoGroup(repo.Id, repo.Name); - MoveSession(sessionName, group.Id); - } - - SaveActiveSessionsToDisk(); - OnStateChanged?.Invoke(); - - // Notify the agent about the directory change so it uses the new path - _ = SendPromptAsync(sessionName, - $"IMPORTANT: Your working directory has changed from `{oldDir}` to `{wt.Path}` (branch: `{wt.Branch}`). " + - $"All file operations should now use this new directory. Please acknowledge.", - skipHistoryMessage: false); - } - /// /// Destroys the existing session and creates a new one with the same name but a different model. /// Use this for "changing" the model of an empty session. From 451f6e6788763fb163a2112df21dac9bcada0830 Mon Sep 17 00:00:00 2001 From: Shane Date: Tue, 24 Feb 2026 16:06:34 -0600 Subject: [PATCH 08/33] Add per-worker worktree isolation for multi-agent groups Adds WorktreeStrategy enum with three options: - Shared: all sessions use one worktree (existing default) - OrchestratorIsolated: orchestrator gets its own, workers share a separate one - FullyIsolated: every session gets its own worktree Implementation: - WorktreeStrategy enum added to SessionOrganization.cs - SessionGroup.WorktreeStrategy persists the chosen strategy - GroupPreset.DefaultWorktreeStrategy sets per-preset defaults - PR Review Squad defaults to FullyIsolated (workers checkout different PRs) - CreateGroupFromPresetAsync auto-creates worktrees based on strategy: - FullyIsolated: creates N worker worktrees in parallel via Task.WhenAll - OrchestratorIsolated: creates 1 orchestrator + 1 shared worker worktree - Shared: no change from current behavior - Orchestrator planning prompt now includes worktree paths for each worker - Worker prompts include worktree awareness note when isolated - DeleteGroup now cleans up all per-session worktrees (previously leaked) All 113 multi-agent tests + 19 organization tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- PolyPilot/Models/ModelCapabilities.cs | 8 +- PolyPilot/Models/SessionOrganization.cs | 19 ++++- .../Services/CopilotService.Organization.cs | 81 ++++++++++++++++--- 3 files changed, 95 insertions(+), 13 deletions(-) diff --git a/PolyPilot/Models/ModelCapabilities.cs b/PolyPilot/Models/ModelCapabilities.cs index c9a608be0b..d63917da5a 100644 --- a/PolyPilot/Models/ModelCapabilities.cs +++ b/PolyPilot/Models/ModelCapabilities.cs @@ -174,6 +174,11 @@ public record GroupPreset(string Name, string Description, string Emoji, MultiAg /// public string? RoutingContext { get; init; } + /// + /// Default worktree allocation strategy for this preset. Null = Shared. + /// + public WorktreeStrategy? DefaultWorktreeStrategy { get; init; } + private const string WorkerReviewPrompt = """ You are a PR reviewer. When assigned a PR, follow this process: @@ -217,7 +222,8 @@ public record GroupPreset(string Name, string Description, string Emoji, MultiAg WorkerReviewPrompt, WorkerReviewPrompt, WorkerReviewPrompt, WorkerReviewPrompt, WorkerReviewPrompt, }, SharedContext = "## Review Standards\n\n- Only flag real issues: bugs, security holes, logic errors, data loss risks, race conditions\n- NEVER comment on style, formatting, naming conventions, or documentation\n- Every finding must include: file path, line number (or range), what's wrong, and why it matters\n- If a PR looks clean, say so — don't invent problems to justify your existence\n- An issue must be flagged by at least 2 of the 5 sub-agent models to be included in the final report (consensus filter)", - RoutingContext = "When given a list of PRs to review, assign ONE PR to EACH worker. Distribute PRs round-robin across the available workers. If there are more PRs than workers, assign multiple PRs per worker.\n\nFor each PR assignment, just tell the worker: \"Review PR #\"\n\nThe workers handle everything else — fetching the diff, dispatching multi-model sub-agents, and synthesizing results. Do NOT micromanage the review process.\n\nAfter all workers complete, produce a brief summary table:\n\n| PR | Verdict | Key Issues |\n|----|---------|------------|\n| #194 | ✅ Ready to merge | None |\n| #193 | ⚠️ Needs changes | Race condition in auth handler |\n\nVerdicts: ✅ Ready to merge, ⚠️ Needs changes, 🔴 Do not merge" + RoutingContext = "When given a list of PRs to review, assign ONE PR to EACH worker. Distribute PRs round-robin across the available workers. If there are more PRs than workers, assign multiple PRs per worker.\n\nFor each PR assignment, just tell the worker: \"Review PR #\"\n\nThe workers handle everything else — fetching the diff, dispatching multi-model sub-agents, and synthesizing results. Do NOT micromanage the review process.\n\nAfter all workers complete, produce a brief summary table:\n\n| PR | Verdict | Key Issues |\n|----|---------|------------|\n| #194 | ✅ Ready to merge | None |\n| #193 | ⚠️ Needs changes | Race condition in auth handler |\n\nVerdicts: ✅ Ready to merge, ⚠️ Needs changes, 🔴 Do not merge", + DefaultWorktreeStrategy = WorktreeStrategy.FullyIsolated }, new GroupPreset( diff --git a/PolyPilot/Models/SessionOrganization.cs b/PolyPilot/Models/SessionOrganization.cs index 9041a32236..59328af99d 100644 --- a/PolyPilot/Models/SessionOrganization.cs +++ b/PolyPilot/Models/SessionOrganization.cs @@ -30,11 +30,14 @@ public class SessionGroup public string? DefaultOrchestratorModel { get; set; } /// - /// Shared worktree for the entire multi-agent group. All sessions use this worktree's path as CWD. - /// Future: per-agent worktrees would move this to SessionMeta and add merge orchestration. + /// Shared worktree for the entire multi-agent group (used as orchestrator worktree + /// when strategy is OrchestratorIsolated or FullyIsolated). /// public string? WorktreeId { get; set; } + /// How worktrees are allocated across sessions in this group. + public WorktreeStrategy WorktreeStrategy { get; set; } = WorktreeStrategy.Shared; + /// Active reflection state for OrchestratorReflect mode. Null when not in a reflect loop. public ReflectionCycle? ReflectionState { get; set; } @@ -99,6 +102,18 @@ public enum MultiAgentMode OrchestratorReflect } +/// How worktrees are allocated across sessions in a multi-agent group. +[JsonConverter(typeof(JsonStringEnumConverter))] +public enum WorktreeStrategy +{ + /// All sessions share one worktree (current default behavior). + Shared, + /// Orchestrator gets its own worktree; all workers share a separate one. + OrchestratorIsolated, + /// Every session (orchestrator + each worker) gets its own worktree. + FullyIsolated +} + /// Role of a session within a multi-agent group. [JsonConverter(typeof(JsonStringEnumConverter))] public enum MultiAgentRole diff --git a/PolyPilot/Services/CopilotService.Organization.cs b/PolyPilot/Services/CopilotService.Organization.cs index 007b9fd121..a2c0571cad 100644 --- a/PolyPilot/Services/CopilotService.Organization.cs +++ b/PolyPilot/Services/CopilotService.Organization.cs @@ -416,6 +416,12 @@ public void DeleteGroup(string groupId) var group = Organization.Groups.FirstOrDefault(g => g.Id == groupId); var isMultiAgent = group?.IsMultiAgent ?? false; + // Collect all worktree IDs for cleanup before removing metadata + var worktreeIds = new HashSet(); + if (group?.WorktreeId != null) worktreeIds.Add(group.WorktreeId); + foreach (var m in Organization.Sessions.Where(m => m.GroupId == groupId)) + if (m.WorktreeId != null) worktreeIds.Add(m.WorktreeId); + if (isMultiAgent) { // Multi-agent sessions are meaningless without their group — close them @@ -441,11 +447,14 @@ public void DeleteGroup(string groupId) // before the fire-and-forget CloseSessionAsync completes SaveActiveSessionsToDisk(); FlushSaveActiveSessionsToDisk(); - // Fire-and-forget: close sessions asynchronously + // Fire-and-forget: close sessions then remove worktrees _ = Task.Run(async () => { foreach (var name in sessionNames) await CloseSessionAsync(name); + // Clean up worktrees after sessions are closed + foreach (var wtId in worktreeIds) + try { await _repoManager.RemoveWorktreeAsync(wtId); } catch { } }); } else @@ -936,10 +945,14 @@ private string BuildOrchestratorPlanningPrompt(string userPrompt, List w { var meta = GetSessionMeta(w); var model = GetEffectiveModel(w); + var wtInfo = meta?.WorktreeId != null + ? _repoManager.Worktrees.FirstOrDefault(wt => wt.Id == meta.WorktreeId) : null; + var desc = $" - '{w}' (model: {model})"; if (!string.IsNullOrEmpty(meta?.SystemPrompt)) - sb.AppendLine($" - '{w}' (model: {model}) — {meta.SystemPrompt}"); - else - sb.AppendLine($" - '{w}' (model: {model})"); + desc += $" — {meta.SystemPrompt}"; + if (wtInfo != null) + desc += $" [isolated worktree: {wtInfo.Path}, branch: {wtInfo.Branch}]"; + sb.AppendLine(desc); } sb.AppendLine(); sb.AppendLine("Route tasks to workers based on their specialization. If a worker has a described role, assign tasks that match their expertise."); @@ -1020,7 +1033,16 @@ private async Task ExecuteWorkerAsync(string workerName, string ta ? $"## Team Context (shared knowledge)\n{group.SharedContext}\n\n" : ""; - var workerPrompt = $"{identity}\n\nYour response will be collected and synthesized with other workers' responses.\n\n{sharedPrefix}## Original User Request (context)\n{originalPrompt}\n\n## Your Assigned Task\n{task}"; + // Inject worktree awareness if the worker has an isolated worktree + var wtInfo = meta?.WorktreeId != null + ? _repoManager.Worktrees.FirstOrDefault(wt => wt.Id == meta.WorktreeId) : null; + var worktreeNote = wtInfo != null && group?.WorktreeStrategy != WorktreeStrategy.Shared + ? $"\n\n## Your Worktree\nYou have an isolated git worktree at `{wtInfo.Path}` (branch: {wtInfo.Branch}). " + + "You can safely run any git operations without affecting other workers. " + + "To check out a PR: `git fetch origin pull//head:pr- && git checkout pr-`\n" + : ""; + + var workerPrompt = $"{identity}{worktreeNote}\n\nYour response will be collected and synthesized with other workers' responses.\n\n{sharedPrefix}## Original User Request (context)\n{originalPrompt}\n\n## Your Assigned Task\n{task}"; try { @@ -1352,13 +1374,51 @@ public string GetEffectiveModel(string sessionName) public async Task CreateGroupFromPresetAsync(Models.GroupPreset preset, string? workingDirectory = null, string? worktreeId = null, string? repoId = null, string? nameOverride = null, CancellationToken ct = default) { var teamName = nameOverride ?? preset.Name; + var strategy = preset.DefaultWorktreeStrategy ?? WorktreeStrategy.Shared; var group = CreateMultiAgentGroup(teamName, preset.Mode, worktreeId: worktreeId, repoId: repoId); if (group == null) return null; + group.WorktreeStrategy = strategy; // Store Squad context (routing, decisions) on the group for use during orchestration group.SharedContext = preset.SharedContext; group.RoutingContext = preset.RoutingContext; + // Determine orchestrator working directory based on strategy + var orchWorkDir = workingDirectory; + var orchWtId = worktreeId; + if (repoId != null && strategy != WorktreeStrategy.Shared && string.IsNullOrEmpty(worktreeId)) + { + // Create a dedicated worktree for the orchestrator + var orchWt = await _repoManager.CreateWorktreeAsync(repoId, $"{teamName}-orchestrator-{Guid.NewGuid().ToString()[..4]}", ct: ct); + orchWorkDir = orchWt.Path; + orchWtId = orchWt.Id; + group.WorktreeId = orchWtId; + } + + // Pre-create worker worktrees in parallel if strategy requires isolation + string?[] workerWorkDirs = new string?[preset.WorkerModels.Length]; + string?[] workerWtIds = new string?[preset.WorkerModels.Length]; + if (repoId != null && strategy == WorktreeStrategy.FullyIsolated) + { + var workerWtTasks = Enumerable.Range(0, preset.WorkerModels.Length).Select(async i => + { + var wt = await _repoManager.CreateWorktreeAsync(repoId, $"{teamName}-worker-{i + 1}-{Guid.NewGuid().ToString()[..4]}", ct: ct); + workerWorkDirs[i] = wt.Path; + workerWtIds[i] = wt.Id; + }); + await Task.WhenAll(workerWtTasks); + } + else if (repoId != null && strategy == WorktreeStrategy.OrchestratorIsolated) + { + // Workers share a single separate worktree + var sharedWorkerWt = await _repoManager.CreateWorktreeAsync(repoId, $"{teamName}-workers-{Guid.NewGuid().ToString()[..4]}", ct: ct); + for (int i = 0; i < preset.WorkerModels.Length; i++) + { + workerWorkDirs[i] = sharedWorkerWt.Path; + workerWtIds[i] = sharedWorkerWt.Id; + } + } + // Create orchestrator session (with uniqueness check matching CreateMultiAgentGroupAsync) var orchName = $"{teamName}-orchestrator"; { int suffix = 1; @@ -1367,7 +1427,7 @@ public string GetEffectiveModel(string sessionName) } try { - await CreateSessionAsync(orchName, preset.OrchestratorModel, workingDirectory, ct); + await CreateSessionAsync(orchName, preset.OrchestratorModel, orchWorkDir, ct); } catch (Exception ex) { @@ -1380,8 +1440,8 @@ public string GetEffectiveModel(string sessionName) // Pin orchestrator so it sorts to the top of the group var orchMeta = GetSessionMeta(orchName); if (orchMeta != null) orchMeta.IsPinned = true; - if (worktreeId != null && orchMeta != null) - orchMeta.WorktreeId = worktreeId; + if (orchWtId != null && orchMeta != null) + orchMeta.WorktreeId = orchWtId; // Create worker sessions for (int i = 0; i < preset.WorkerModels.Length; i++) @@ -1392,9 +1452,10 @@ public string GetEffectiveModel(string sessionName) workerName = $"{teamName}-worker-{i + 1}-{suffix++}"; } var workerModel = preset.WorkerModels[i]; + var workerWorkDir = workerWorkDirs[i] ?? workingDirectory; try { - await CreateSessionAsync(workerName, workerModel, workingDirectory, ct); + await CreateSessionAsync(workerName, workerModel, workerWorkDir, ct); } catch (Exception ex) { @@ -1408,7 +1469,7 @@ public string GetEffectiveModel(string sessionName) var meta = GetSessionMeta(workerName); if (meta != null) { - if (worktreeId != null) meta.WorktreeId = worktreeId; + meta.WorktreeId = workerWtIds[i] ?? worktreeId; if (systemPrompt != null) meta.SystemPrompt = systemPrompt; } } From 7782c1296cb9561ecd7807d688ab14491fef83af Mon Sep 17 00:00:00 2001 From: Shane Date: Tue, 24 Feb 2026 16:22:01 -0600 Subject: [PATCH 09/33] Replace worktree picker with repo picker for multi-agent creation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since worktrees are now auto-created based on WorktreeStrategy, the 'Select worktree for team' step is replaced with 'Select repository for team'. Users pick a repo, then a preset — worktrees are created automatically (FullyIsolated creates one per agent, etc). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Components/Layout/SessionSidebar.razor | 74 ++++++++++--------- 1 file changed, 38 insertions(+), 36 deletions(-) diff --git a/PolyPilot/Components/Layout/SessionSidebar.razor b/PolyPilot/Components/Layout/SessionSidebar.razor index abcf78b701..015283ad69 100644 --- a/PolyPilot/Components/Layout/SessionSidebar.razor +++ b/PolyPilot/Components/Layout/SessionSidebar.razor @@ -204,49 +204,49 @@ else - @if (isAddingGroup && !isAddingMultiAgentGroup && pendingMultiAgentWorktree == null) + @if (isAddingGroup && !isAddingMultiAgentGroup && pendingMultiAgentRepo == null) { } - else if (isAddingMultiAgentGroup && pendingMultiAgentWorktree == null) + else if (isAddingMultiAgentGroup && pendingMultiAgentRepo == null) { - @* Step 1: Pick a worktree *@ + @* Step 1: Pick a repository *@
- Select worktree for team: + Select repository for team:
- @foreach (var wt in RepoManager.Worktrees) + @foreach (var repo in RepoManager.Repositories) { - var w = wt; - var repo = RepoManager.Repositories.FirstOrDefault(r => r.Id == w.RepoId); - } - @if (!RepoManager.Worktrees.Any()) + @if (!RepoManager.Repositories.Any()) { -
No worktrees available. Add a repository first.
+
No repositories available. Add a repository first.
}
} - else if (pendingMultiAgentWorktree != null) + else if (pendingMultiAgentRepo != null) { @* Step 2: Pick a preset or enter a custom name *@
- 🌿 @pendingMultiAgentWorktree.Branch + 📦 @pendingMultiAgentRepo.Name
@{ - var allPresets = UserPresets.GetAll(CopilotService.BaseDir, pendingMultiAgentWorktree?.Path); + var repoWorktree = RepoManager.Worktrees.FirstOrDefault(w => w.RepoId == pendingMultiAgentRepo?.Id); + var allPresets = UserPresets.GetAll(CopilotService.BaseDir, repoWorktree?.Path); var repoPresets = allPresets.Where(p => p.IsRepoLevel).ToArray(); var builtInPresets = allPresets.Where(p => !p.IsRepoLevel && !p.IsUserDefined).ToArray(); var userPresets = allPresets.Where(p => p.IsUserDefined).ToArray(); @@ -257,7 +257,7 @@ else @foreach (var preset in repoPresets) { var p = preset; - @@ -285,7 +285,7 @@ else @foreach (var preset in userPresets) { var p = preset; - @@ -1314,40 +1314,43 @@ else { isAddingGroup = false; isAddingMultiAgentGroup = true; - pendingMultiAgentWorktree = null; + pendingMultiAgentRepo = null; } private void CancelMultiAgentCreation() { isAddingMultiAgentGroup = false; - pendingMultiAgentWorktree = null; + pendingMultiAgentRepo = null; } - private async Task SelectWorktreeForGroup(WorktreeInfo wt) + private void SelectRepoForGroup(RepositoryInfo repo) { - // Worktree selected — advance to step 2 (presets + custom name) - pendingMultiAgentWorktree = wt; + // Repo selected — advance to step 2 (presets + custom name) + pendingMultiAgentRepo = repo; isAddingMultiAgentGroup = false; isAddingGroup = false; StateHasChanged(); } - private WorktreeInfo? pendingMultiAgentWorktree; + private RepositoryInfo? pendingMultiAgentRepo; - private async Task CreateFromPresetWithWorktree(GroupPreset preset, WorktreeInfo wt) + private async Task CreateFromPresetForRepo(GroupPreset preset, RepositoryInfo repo) { // Read optional custom name from the input var customName = await JS.InvokeAsync("getElementValue", "presetNameInput"); var nameOverride = string.IsNullOrWhiteSpace(customName) ? null : customName.Trim(); - pendingMultiAgentWorktree = null; + // Find an existing worktree for fallback working directory (Shared strategy) + var fallbackWt = RepoManager.Worktrees.FirstOrDefault(w => w.RepoId == repo.Id); + + pendingMultiAgentRepo = null; StateHasChanged(); try { await CopilotService.CreateGroupFromPresetAsync(preset, - workingDirectory: wt.Path, - worktreeId: wt.Id, - repoId: wt.RepoId, + workingDirectory: fallbackWt?.Path, + worktreeId: null, + repoId: repo.Id, nameOverride: nameOverride); } catch (Exception ex) @@ -1451,18 +1454,17 @@ else private async Task CommitNewGroup() { var name = await JS.InvokeAsync("getElementValue", "newGroupInput"); - var wt = pendingMultiAgentWorktree; + var repo = pendingMultiAgentRepo; isAddingGroup = false; isAddingMultiAgentGroup = false; - pendingMultiAgentWorktree = null; + pendingMultiAgentRepo = null; if (!string.IsNullOrWhiteSpace(name)) { - if (wt != null) + if (repo != null) { - // Multi-agent group with worktree + // Multi-agent group with repo CopilotService.CreateMultiAgentGroup(name.Trim(), - worktreeId: wt.Id, - repoId: wt.RepoId); + repoId: repo.Id); } else { @@ -1474,7 +1476,7 @@ else private async Task HandleNewGroupKeyDown(KeyboardEventArgs e) { if (e.Key == "Enter") await CommitNewGroup(); - else if (e.Key == "Escape") { isAddingGroup = false; isAddingMultiAgentGroup = false; pendingMultiAgentWorktree = null; } + else if (e.Key == "Escape") { isAddingGroup = false; isAddingMultiAgentGroup = false; pendingMultiAgentRepo = null; } } private void ToggleSessionMenu(string sessionName) From 332e0fc05232e461752b58d0308d63eb6b8c7830 Mon Sep 17 00:00:00 2001 From: Shane Date: Tue, 24 Feb 2026 16:29:36 -0600 Subject: [PATCH 10/33] Add worktree strategy selector and fix multi-agent session creation Two fixes: 1. Added a dropdown to select worktree strategy (FullyIsolated / OrchestratorIsolated / Shared) when creating multi-agent groups. Defaults to FullyIsolated. 2. Fixed session creation failure: parallel git worktree creation caused lock contention on bare repos. Now does one fetch upfront then creates worktrees sequentially with skipFetch flag. Also fixed two callers of CreateWorktreeAsync that broke when the skipFetch parameter was added (positional CancellationToken arg). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Components/Layout/SessionSidebar.razor | 20 ++++++++++++++- .../Services/CopilotService.Organization.cs | 25 ++++++++++++------- PolyPilot/Services/CopilotService.cs | 2 +- PolyPilot/Services/RepoManager.cs | 5 ++-- PolyPilot/Services/WsBridgeServer.cs | 2 +- 5 files changed, 40 insertions(+), 14 deletions(-) diff --git a/PolyPilot/Components/Layout/SessionSidebar.razor b/PolyPilot/Components/Layout/SessionSidebar.razor index 015283ad69..7ba6f22b34 100644 --- a/PolyPilot/Components/Layout/SessionSidebar.razor +++ b/PolyPilot/Components/Layout/SessionSidebar.razor @@ -244,6 +244,14 @@ else +
+ Worktrees: + +
@{ var repoWorktree = RepoManager.Worktrees.FirstOrDefault(w => w.RepoId == pendingMultiAgentRepo?.Id); var allPresets = UserPresets.GetAll(CopilotService.BaseDir, repoWorktree?.Path); @@ -1333,6 +1341,13 @@ else } private RepositoryInfo? pendingMultiAgentRepo; + private WorktreeStrategy selectedStrategy = WorktreeStrategy.FullyIsolated; + + private void OnStrategyChanged(ChangeEventArgs e) + { + if (Enum.TryParse(e.Value?.ToString(), out var s)) + selectedStrategy = s; + } private async Task CreateFromPresetForRepo(GroupPreset preset, RepositoryInfo repo) { @@ -1343,7 +1358,9 @@ else // Find an existing worktree for fallback working directory (Shared strategy) var fallbackWt = RepoManager.Worktrees.FirstOrDefault(w => w.RepoId == repo.Id); + var strategyOverride = selectedStrategy; pendingMultiAgentRepo = null; + selectedStrategy = WorktreeStrategy.FullyIsolated; // reset for next time StateHasChanged(); try { @@ -1351,7 +1368,8 @@ else workingDirectory: fallbackWt?.Path, worktreeId: null, repoId: repo.Id, - nameOverride: nameOverride); + nameOverride: nameOverride, + strategyOverride: strategyOverride); } catch (Exception ex) { diff --git a/PolyPilot/Services/CopilotService.Organization.cs b/PolyPilot/Services/CopilotService.Organization.cs index a2c0571cad..f3c8624929 100644 --- a/PolyPilot/Services/CopilotService.Organization.cs +++ b/PolyPilot/Services/CopilotService.Organization.cs @@ -1371,10 +1371,10 @@ public string GetEffectiveModel(string sessionName) /// /// Create a multi-agent group from a preset template, creating sessions with assigned models. /// - public async Task CreateGroupFromPresetAsync(Models.GroupPreset preset, string? workingDirectory = null, string? worktreeId = null, string? repoId = null, string? nameOverride = null, CancellationToken ct = default) + public async Task CreateGroupFromPresetAsync(Models.GroupPreset preset, string? workingDirectory = null, string? worktreeId = null, string? repoId = null, string? nameOverride = null, WorktreeStrategy? strategyOverride = null, CancellationToken ct = default) { var teamName = nameOverride ?? preset.Name; - var strategy = preset.DefaultWorktreeStrategy ?? WorktreeStrategy.Shared; + var strategy = strategyOverride ?? preset.DefaultWorktreeStrategy ?? WorktreeStrategy.Shared; var group = CreateMultiAgentGroup(teamName, preset.Mode, worktreeId: worktreeId, repoId: repoId); if (group == null) return null; group.WorktreeStrategy = strategy; @@ -1386,32 +1386,39 @@ public string GetEffectiveModel(string sessionName) // Determine orchestrator working directory based on strategy var orchWorkDir = workingDirectory; var orchWtId = worktreeId; + + // Pre-fetch once to avoid parallel git lock contention + if (repoId != null && strategy != WorktreeStrategy.Shared) + { + try { await _repoManager.FetchAsync(repoId, ct); } + catch (Exception ex) { Debug($"Pre-fetch failed (continuing): {ex.Message}"); } + } + if (repoId != null && strategy != WorktreeStrategy.Shared && string.IsNullOrEmpty(worktreeId)) { // Create a dedicated worktree for the orchestrator - var orchWt = await _repoManager.CreateWorktreeAsync(repoId, $"{teamName}-orchestrator-{Guid.NewGuid().ToString()[..4]}", ct: ct); + var orchWt = await _repoManager.CreateWorktreeAsync(repoId, $"{teamName}-orchestrator-{Guid.NewGuid().ToString()[..4]}", skipFetch: true, ct: ct); orchWorkDir = orchWt.Path; orchWtId = orchWt.Id; group.WorktreeId = orchWtId; } - // Pre-create worker worktrees in parallel if strategy requires isolation + // Pre-create worker worktrees sequentially (git worktree add uses locks on bare repos) string?[] workerWorkDirs = new string?[preset.WorkerModels.Length]; string?[] workerWtIds = new string?[preset.WorkerModels.Length]; if (repoId != null && strategy == WorktreeStrategy.FullyIsolated) { - var workerWtTasks = Enumerable.Range(0, preset.WorkerModels.Length).Select(async i => + for (int i = 0; i < preset.WorkerModels.Length; i++) { - var wt = await _repoManager.CreateWorktreeAsync(repoId, $"{teamName}-worker-{i + 1}-{Guid.NewGuid().ToString()[..4]}", ct: ct); + var wt = await _repoManager.CreateWorktreeAsync(repoId, $"{teamName}-worker-{i + 1}-{Guid.NewGuid().ToString()[..4]}", skipFetch: true, ct: ct); workerWorkDirs[i] = wt.Path; workerWtIds[i] = wt.Id; - }); - await Task.WhenAll(workerWtTasks); + } } else if (repoId != null && strategy == WorktreeStrategy.OrchestratorIsolated) { // Workers share a single separate worktree - var sharedWorkerWt = await _repoManager.CreateWorktreeAsync(repoId, $"{teamName}-workers-{Guid.NewGuid().ToString()[..4]}", ct: ct); + var sharedWorkerWt = await _repoManager.CreateWorktreeAsync(repoId, $"{teamName}-workers-{Guid.NewGuid().ToString()[..4]}", skipFetch: true, ct: ct); for (int i = 0; i < preset.WorkerModels.Length; i++) { workerWorkDirs[i] = sharedWorkerWt.Path; diff --git a/PolyPilot/Services/CopilotService.cs b/PolyPilot/Services/CopilotService.cs index 01ab421272..e68bb229b4 100644 --- a/PolyPilot/Services/CopilotService.cs +++ b/PolyPilot/Services/CopilotService.cs @@ -1437,7 +1437,7 @@ public async Task CreateSessionWithWorktreeAsync( else { var branch = branchName ?? $"session-{DateTime.Now:yyyyMMdd-HHmmss}"; - wt = await _repoManager.CreateWorktreeAsync(repoId, branch, null, ct); + wt = await _repoManager.CreateWorktreeAsync(repoId, branch, null, ct: ct); } var name = sessionName ?? wt.Branch; diff --git a/PolyPilot/Services/RepoManager.cs b/PolyPilot/Services/RepoManager.cs index 5974921366..5ddbad011d 100644 --- a/PolyPilot/Services/RepoManager.cs +++ b/PolyPilot/Services/RepoManager.cs @@ -206,14 +206,15 @@ public async Task AddRepositoryFromLocalAsync(string localPath, /// /// Create a new worktree for a repository on a new branch from origin/main. /// - public async Task CreateWorktreeAsync(string repoId, string branchName, string? baseBranch = null, CancellationToken ct = default) + public async Task CreateWorktreeAsync(string repoId, string branchName, string? baseBranch = null, bool skipFetch = false, CancellationToken ct = default) { EnsureLoaded(); var repo = _state.Repositories.FirstOrDefault(r => r.Id == repoId) ?? throw new InvalidOperationException($"Repository '{repoId}' not found."); // Fetch latest from origin (prune to clean up deleted remote branches) - await RunGitAsync(repo.BareClonePath, ct, "fetch", "--prune", "origin"); + if (!skipFetch) + await RunGitAsync(repo.BareClonePath, ct, "fetch", "--prune", "origin"); // Determine base ref var baseRef = baseBranch ?? await GetDefaultBranch(repo.BareClonePath, ct); diff --git a/PolyPilot/Services/WsBridgeServer.cs b/PolyPilot/Services/WsBridgeServer.cs index 6165be980c..ff363fa99f 100644 --- a/PolyPilot/Services/WsBridgeServer.cs +++ b/PolyPilot/Services/WsBridgeServer.cs @@ -711,7 +711,7 @@ await SendToClientAsync(clientId, ws, if (wtReq.PrNumber.HasValue) wt = await _repoManager.CreateWorktreeFromPrAsync(wtReq.RepoId, wtReq.PrNumber.Value, ct); else - wt = await _repoManager.CreateWorktreeAsync(wtReq.RepoId, wtReq.BranchName ?? "main", null, ct); + wt = await _repoManager.CreateWorktreeAsync(wtReq.RepoId, wtReq.BranchName ?? "main", null, ct: ct); await SendToClientAsync(clientId, ws, BridgeMessage.Create(BridgeMessageTypes.WorktreeCreated, new WorktreeCreatedPayload From 496daed1575b8f82d31ff63964ad558a02e07e81 Mon Sep 17 00:00:00 2001 From: Shane Date: Wed, 25 Feb 2026 08:39:08 -0600 Subject: [PATCH 11/33] =?UTF-8?q?Make=20worktree=20creation=20resilient=20?= =?UTF-8?q?=E2=80=94=20fall=20back=20to=20shared=20on=20failure?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wrap all worktree creation calls in try-catch so that if git worktree add fails (e.g., lock contention, disk issues, large repos), session creation still proceeds with the fallback shared working directory. Previously, a single worktree failure would abort the entire CreateGroupFromPresetAsync, leaving an empty group with no sessions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Services/CopilotService.Organization.cs | 45 +++++++++++++------ 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/PolyPilot/Services/CopilotService.Organization.cs b/PolyPilot/Services/CopilotService.Organization.cs index f3c8624929..7181eb1cbc 100644 --- a/PolyPilot/Services/CopilotService.Organization.cs +++ b/PolyPilot/Services/CopilotService.Organization.cs @@ -1396,11 +1396,17 @@ public string GetEffectiveModel(string sessionName) if (repoId != null && strategy != WorktreeStrategy.Shared && string.IsNullOrEmpty(worktreeId)) { - // Create a dedicated worktree for the orchestrator - var orchWt = await _repoManager.CreateWorktreeAsync(repoId, $"{teamName}-orchestrator-{Guid.NewGuid().ToString()[..4]}", skipFetch: true, ct: ct); - orchWorkDir = orchWt.Path; - orchWtId = orchWt.Id; - group.WorktreeId = orchWtId; + try + { + var orchWt = await _repoManager.CreateWorktreeAsync(repoId, $"{teamName}-orchestrator-{Guid.NewGuid().ToString()[..4]}", skipFetch: true, ct: ct); + orchWorkDir = orchWt.Path; + orchWtId = orchWt.Id; + group.WorktreeId = orchWtId; + } + catch (Exception ex) + { + Debug($"Failed to create orchestrator worktree (falling back to shared): {ex.Message}"); + } } // Pre-create worker worktrees sequentially (git worktree add uses locks on bare repos) @@ -1410,19 +1416,32 @@ public string GetEffectiveModel(string sessionName) { for (int i = 0; i < preset.WorkerModels.Length; i++) { - var wt = await _repoManager.CreateWorktreeAsync(repoId, $"{teamName}-worker-{i + 1}-{Guid.NewGuid().ToString()[..4]}", skipFetch: true, ct: ct); - workerWorkDirs[i] = wt.Path; - workerWtIds[i] = wt.Id; + try + { + var wt = await _repoManager.CreateWorktreeAsync(repoId, $"{teamName}-worker-{i + 1}-{Guid.NewGuid().ToString()[..4]}", skipFetch: true, ct: ct); + workerWorkDirs[i] = wt.Path; + workerWtIds[i] = wt.Id; + } + catch (Exception ex) + { + Debug($"Failed to create worker-{i + 1} worktree (falling back to shared): {ex.Message}"); + } } } else if (repoId != null && strategy == WorktreeStrategy.OrchestratorIsolated) { - // Workers share a single separate worktree - var sharedWorkerWt = await _repoManager.CreateWorktreeAsync(repoId, $"{teamName}-workers-{Guid.NewGuid().ToString()[..4]}", skipFetch: true, ct: ct); - for (int i = 0; i < preset.WorkerModels.Length; i++) + try + { + var sharedWorkerWt = await _repoManager.CreateWorktreeAsync(repoId, $"{teamName}-workers-{Guid.NewGuid().ToString()[..4]}", skipFetch: true, ct: ct); + for (int i = 0; i < preset.WorkerModels.Length; i++) + { + workerWorkDirs[i] = sharedWorkerWt.Path; + workerWtIds[i] = sharedWorkerWt.Id; + } + } + catch (Exception ex) { - workerWorkDirs[i] = sharedWorkerWt.Path; - workerWtIds[i] = sharedWorkerWt.Id; + Debug($"Failed to create shared worker worktree (falling back to shared): {ex.Message}"); } } From 3cd832b394c01a50d4a24e7a0adc93126c24ac50 Mon Sep 17 00:00:00 2001 From: Shane Date: Wed, 25 Feb 2026 09:05:14 -0600 Subject: [PATCH 12/33] Make worktree strategy selector more prominent in multi-agent creation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Added a bordered card with header '🌿 Worktree Isolation' around the strategy dropdown so it's clearly visible between the team name input and the preset list. Previously it was a subtle inline element that was easy to miss. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- PolyPilot/Components/Layout/SessionSidebar.razor | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/PolyPilot/Components/Layout/SessionSidebar.razor b/PolyPilot/Components/Layout/SessionSidebar.razor index 7ba6f22b34..f9fa0cbe49 100644 --- a/PolyPilot/Components/Layout/SessionSidebar.razor +++ b/PolyPilot/Components/Layout/SessionSidebar.razor @@ -244,9 +244,9 @@ else -
- Worktrees: - From 49ebf98c1a5b0d6d3148a81519eafaf331450335 Mon Sep 17 00:00:00 2001 From: Shane Date: Wed, 25 Feb 2026 09:14:20 -0600 Subject: [PATCH 13/33] Add 20 worktree strategy tests and diagnostic logging New WorktreeStrategyTests.cs covers: - FullyIsolated: unique worktree per session, skip-fetch optimization - OrchestratorIsolated: 2 worktrees, workers share one - Shared: no worktrees created - StrategyOverride: UI override takes precedence over preset default - Error resilience: sessions still created when worktrees fail - Session correctness: right count, orchestrator pinned, group wt ID Also: - Made RepoManager.CreateWorktreeAsync/FetchAsync virtual for testing - Added diagnostic logs showing worktree strategy, directories, and per-worker worktree assignments for debugging Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- PolyPilot.Tests/WorktreeStrategyTests.cs | 497 ++++++++++++++++++ .../Services/CopilotService.Organization.cs | 5 + PolyPilot/Services/RepoManager.cs | 4 +- 3 files changed, 504 insertions(+), 2 deletions(-) create mode 100644 PolyPilot.Tests/WorktreeStrategyTests.cs diff --git a/PolyPilot.Tests/WorktreeStrategyTests.cs b/PolyPilot.Tests/WorktreeStrategyTests.cs new file mode 100644 index 0000000000..800f5ee303 --- /dev/null +++ b/PolyPilot.Tests/WorktreeStrategyTests.cs @@ -0,0 +1,497 @@ +using System.Text.Json; +using Microsoft.Extensions.DependencyInjection; +using PolyPilot.Models; +using PolyPilot.Services; + +namespace PolyPilot.Tests; + +/// +/// Tests for per-worker worktree isolation strategies in multi-agent groups. +/// Validates that CreateGroupFromPresetAsync creates the correct number of +/// worktrees with unique paths per strategy, and that session metadata is +/// correctly wired up. +/// +public class WorktreeStrategyTests +{ + private readonly StubChatDatabase _chatDb = new(); + private readonly StubServerManager _serverManager = new(); + private readonly StubWsBridgeClient _bridgeClient = new(); + private readonly StubDemoService _demoService = new(); + private readonly IServiceProvider _serviceProvider; + + public WorktreeStrategyTests() + { + var services = new ServiceCollection(); + _serviceProvider = services.BuildServiceProvider(); + } + + /// + /// A FakeRepoManager that doesn't touch git — returns fake worktrees + /// with unique IDs and paths, tracking all creation calls. + /// + private class FakeRepoManager : RepoManager + { + public List<(string RepoId, string BranchName, bool SkipFetch)> CreateCalls { get; } = new(); + public int FetchCallCount { get; private set; } + private int _worktreeCounter; + + public FakeRepoManager(List repos) + { + // Inject state via reflection (same pattern as existing tests) + 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 = new() }); + loadedField.SetValue(this, true); + } + + public override Task CreateWorktreeAsync(string repoId, string branchName, + string? baseBranch = null, bool skipFetch = false, CancellationToken ct = default) + { + CreateCalls.Add((repoId, branchName, skipFetch)); + var id = $"wt-{Interlocked.Increment(ref _worktreeCounter)}"; + var wt = new WorktreeInfo + { + Id = id, + RepoId = repoId, + Branch = branchName, + Path = $"/fake/worktrees/{id}" + }; + return Task.FromResult(wt); + } + + public override Task FetchAsync(string repoId, CancellationToken ct = default) + { + FetchCallCount++; + return Task.CompletedTask; + } + } + + private CopilotService CreateDemoService(RepoManager rm) + { + var svc = new CopilotService(_chatDb, _serverManager, _bridgeClient, rm, _serviceProvider, _demoService); + // Enable demo mode so CreateSessionAsync works without a real Copilot client + var prop = typeof(CopilotService).GetProperty("IsDemoMode")!; + prop.SetValue(svc, true); + return svc; + } + + private static GroupPreset MakePreset(int workerCount, WorktreeStrategy? defaultStrategy = null) + { + return new GroupPreset( + "TestTeam", "Test", "🧪", MultiAgentMode.Orchestrator, + "claude-opus-4.6", Enumerable.Repeat("claude-sonnet-4.6", workerCount).ToArray()) + { + DefaultWorktreeStrategy = defaultStrategy + }; + } + + #region WorktreeStrategy Enum Serialization + + [Fact] + public void WorktreeStrategy_AllValues_RoundTrip() + { + foreach (var strategy in Enum.GetValues()) + { + var group = new SessionGroup + { + Id = $"test-{strategy}", + Name = $"Test {strategy}", + IsMultiAgent = true, + WorktreeStrategy = strategy + }; + + var json = JsonSerializer.Serialize(group); + var restored = JsonSerializer.Deserialize(json)!; + + Assert.Equal(strategy, restored.WorktreeStrategy); + } + } + + [Fact] + public void WorktreeStrategy_DefaultsToShared() + { + var group = new SessionGroup { Id = "x", Name = "X" }; + Assert.Equal(WorktreeStrategy.Shared, group.WorktreeStrategy); + } + + [Fact] + public void GroupPreset_DefaultWorktreeStrategy_Nullable() + { + var preset = new GroupPreset("T", "D", "E", MultiAgentMode.Broadcast, "m", new[] { "w" }); + Assert.Null(preset.DefaultWorktreeStrategy); + + var presetWithStrategy = new GroupPreset("T", "D", "E", MultiAgentMode.Broadcast, "m", new[] { "w" }) + { + DefaultWorktreeStrategy = WorktreeStrategy.FullyIsolated + }; + Assert.Equal(WorktreeStrategy.FullyIsolated, presetWithStrategy.DefaultWorktreeStrategy); + } + + [Fact] + public void PRReviewSquad_DefaultsToFullyIsolated() + { + var prSquad = GroupPreset.BuiltIn.First(p => p.Name == "PR Review Squad"); + Assert.Equal(WorktreeStrategy.FullyIsolated, prSquad.DefaultWorktreeStrategy); + } + + #endregion + + #region FullyIsolated Strategy + + [Fact] + public async Task FullyIsolated_CreatesUniqueWorktreePerSession() + { + var rm = new FakeRepoManager(new() { new() { Id = "repo-1", Name = "Repo" } }); + var svc = CreateDemoService(rm); + var preset = MakePreset(3, WorktreeStrategy.FullyIsolated); + + var group = await svc.CreateGroupFromPresetAsync(preset, + workingDirectory: "/fallback", + repoId: "repo-1"); + + Assert.NotNull(group); + Assert.Equal(WorktreeStrategy.FullyIsolated, group!.WorktreeStrategy); + + // 1 orchestrator + 3 workers = 4 worktrees + Assert.Equal(4, rm.CreateCalls.Count); + } + + [Fact] + public async Task FullyIsolated_AllWorktreeIdsAreUnique() + { + var rm = new FakeRepoManager(new() { new() { Id = "repo-1", Name = "Repo" } }); + var svc = CreateDemoService(rm); + var preset = MakePreset(3, WorktreeStrategy.FullyIsolated); + + var group = await svc.CreateGroupFromPresetAsync(preset, + workingDirectory: "/fallback", + repoId: "repo-1"); + + // Get all session worktree IDs + var worktreeIds = svc.Organization.Sessions + .Where(s => s.GroupId == group!.Id) + .Select(s => s.WorktreeId) + .ToList(); + + // All should be non-null and unique + Assert.All(worktreeIds, id => Assert.NotNull(id)); + Assert.Equal(worktreeIds.Count, worktreeIds.Distinct().Count()); + } + + [Fact] + public async Task FullyIsolated_OrchestratorHasDifferentWorktreeThanWorkers() + { + var rm = new FakeRepoManager(new() { new() { Id = "repo-1", Name = "Repo" } }); + var svc = CreateDemoService(rm); + var preset = MakePreset(2, WorktreeStrategy.FullyIsolated); + + var group = await svc.CreateGroupFromPresetAsync(preset, + workingDirectory: "/fallback", + repoId: "repo-1"); + + var orchMeta = svc.Organization.Sessions + .First(s => s.GroupId == group!.Id && s.Role == MultiAgentRole.Orchestrator); + var workerMetas = svc.Organization.Sessions + .Where(s => s.GroupId == group!.Id && s.Role != MultiAgentRole.Orchestrator) + .ToList(); + + Assert.NotNull(orchMeta.WorktreeId); + Assert.All(workerMetas, w => + { + Assert.NotNull(w.WorktreeId); + Assert.NotEqual(orchMeta.WorktreeId, w.WorktreeId); + }); + } + + [Fact] + public async Task FullyIsolated_SkipsFetchOnWorkerWorktrees() + { + var rm = new FakeRepoManager(new() { new() { Id = "repo-1", Name = "Repo" } }); + var svc = CreateDemoService(rm); + var preset = MakePreset(3, WorktreeStrategy.FullyIsolated); + + await svc.CreateGroupFromPresetAsync(preset, + workingDirectory: "/fallback", + repoId: "repo-1"); + + // One explicit FetchAsync call upfront + Assert.Equal(1, rm.FetchCallCount); + // All CreateWorktreeAsync calls should have skipFetch=true + Assert.All(rm.CreateCalls, c => Assert.True(c.SkipFetch)); + } + + #endregion + + #region OrchestratorIsolated Strategy + + [Fact] + public async Task OrchestratorIsolated_Creates2Worktrees() + { + var rm = new FakeRepoManager(new() { new() { Id = "repo-1", Name = "Repo" } }); + var svc = CreateDemoService(rm); + var preset = MakePreset(3, WorktreeStrategy.OrchestratorIsolated); + + var group = await svc.CreateGroupFromPresetAsync(preset, + workingDirectory: "/fallback", + repoId: "repo-1"); + + Assert.NotNull(group); + Assert.Equal(WorktreeStrategy.OrchestratorIsolated, group!.WorktreeStrategy); + + // 1 orchestrator + 1 shared worker = 2 worktrees + Assert.Equal(2, rm.CreateCalls.Count); + } + + [Fact] + public async Task OrchestratorIsolated_WorkersShareSameWorktree() + { + var rm = new FakeRepoManager(new() { new() { Id = "repo-1", Name = "Repo" } }); + var svc = CreateDemoService(rm); + var preset = MakePreset(3, WorktreeStrategy.OrchestratorIsolated); + + var group = await svc.CreateGroupFromPresetAsync(preset, + workingDirectory: "/fallback", + repoId: "repo-1"); + + var workerMetas = svc.Organization.Sessions + .Where(s => s.GroupId == group!.Id && s.Role != MultiAgentRole.Orchestrator) + .ToList(); + + Assert.Equal(3, workerMetas.Count); + // All workers should share the same worktree ID + var workerWtIds = workerMetas.Select(w => w.WorktreeId).Distinct().ToList(); + Assert.Single(workerWtIds); + Assert.NotNull(workerWtIds[0]); + } + + [Fact] + public async Task OrchestratorIsolated_OrchestratorHasDifferentWorktreeThanWorkers() + { + var rm = new FakeRepoManager(new() { new() { Id = "repo-1", Name = "Repo" } }); + var svc = CreateDemoService(rm); + var preset = MakePreset(2, WorktreeStrategy.OrchestratorIsolated); + + var group = await svc.CreateGroupFromPresetAsync(preset, + workingDirectory: "/fallback", + repoId: "repo-1"); + + var orchMeta = svc.Organization.Sessions + .First(s => s.GroupId == group!.Id && s.Role == MultiAgentRole.Orchestrator); + var workerMeta = svc.Organization.Sessions + .First(s => s.GroupId == group!.Id && s.Role != MultiAgentRole.Orchestrator); + + Assert.NotNull(orchMeta.WorktreeId); + Assert.NotNull(workerMeta.WorktreeId); + Assert.NotEqual(orchMeta.WorktreeId, workerMeta.WorktreeId); + } + + #endregion + + #region Shared Strategy + + [Fact] + public async Task Shared_CreatesNoWorktrees() + { + var rm = new FakeRepoManager(new() { new() { Id = "repo-1", Name = "Repo" } }); + var svc = CreateDemoService(rm); + var preset = MakePreset(3, WorktreeStrategy.Shared); + + var group = await svc.CreateGroupFromPresetAsync(preset, + workingDirectory: "/fallback", + repoId: "repo-1"); + + Assert.NotNull(group); + Assert.Equal(WorktreeStrategy.Shared, group!.WorktreeStrategy); + + // No worktrees created + Assert.Empty(rm.CreateCalls); + Assert.Equal(0, rm.FetchCallCount); + } + + [Fact] + public async Task Shared_AllSessionsGetNullWorktreeId() + { + var rm = new FakeRepoManager(new() { new() { Id = "repo-1", Name = "Repo" } }); + var svc = CreateDemoService(rm); + var preset = MakePreset(2, WorktreeStrategy.Shared); + + var group = await svc.CreateGroupFromPresetAsync(preset, + workingDirectory: "/fallback", + repoId: "repo-1"); + + var allMetas = svc.Organization.Sessions + .Where(s => s.GroupId == group!.Id) + .ToList(); + + Assert.Equal(3, allMetas.Count); // 1 orch + 2 workers + Assert.All(allMetas, m => Assert.Null(m.WorktreeId)); + } + + #endregion + + #region StrategyOverride + + [Fact] + public async Task StrategyOverride_OverridesPresetDefault() + { + var rm = new FakeRepoManager(new() { new() { Id = "repo-1", Name = "Repo" } }); + var svc = CreateDemoService(rm); + // Preset defaults to FullyIsolated + var preset = MakePreset(2, WorktreeStrategy.FullyIsolated); + + // Override to Shared + var group = await svc.CreateGroupFromPresetAsync(preset, + workingDirectory: "/fallback", + repoId: "repo-1", + strategyOverride: WorktreeStrategy.Shared); + + Assert.Equal(WorktreeStrategy.Shared, group!.WorktreeStrategy); + Assert.Empty(rm.CreateCalls); + } + + [Fact] + public async Task StrategyOverride_NullUsesPresetDefault() + { + var rm = new FakeRepoManager(new() { new() { Id = "repo-1", Name = "Repo" } }); + var svc = CreateDemoService(rm); + var preset = MakePreset(2, WorktreeStrategy.FullyIsolated); + + var group = await svc.CreateGroupFromPresetAsync(preset, + workingDirectory: "/fallback", + repoId: "repo-1", + strategyOverride: null); + + Assert.Equal(WorktreeStrategy.FullyIsolated, group!.WorktreeStrategy); + Assert.Equal(3, rm.CreateCalls.Count); // 1 orch + 2 workers + } + + [Fact] + public async Task NoRepoId_SkipsWorktreeCreation() + { + var rm = new FakeRepoManager(new() { new() { Id = "repo-1", Name = "Repo" } }); + var svc = CreateDemoService(rm); + var preset = MakePreset(2, WorktreeStrategy.FullyIsolated); + + // No repoId — can't create worktrees + var group = await svc.CreateGroupFromPresetAsync(preset, + workingDirectory: "/fallback", + repoId: null); + + Assert.NotNull(group); + Assert.Empty(rm.CreateCalls); + } + + #endregion + + #region Session Creation Correctness + + [Fact] + public async Task FullyIsolated_CorrectNumberOfSessions() + { + var rm = new FakeRepoManager(new() { new() { Id = "repo-1", Name = "Repo" } }); + var svc = CreateDemoService(rm); + var preset = MakePreset(5, WorktreeStrategy.FullyIsolated); + + var group = await svc.CreateGroupFromPresetAsync(preset, + workingDirectory: "/fallback", + repoId: "repo-1"); + + var members = svc.Organization.Sessions + .Where(s => s.GroupId == group!.Id) + .ToList(); + + Assert.Equal(6, members.Count); // 1 orchestrator + 5 workers + Assert.Single(members.Where(m => m.Role == MultiAgentRole.Orchestrator)); + } + + [Fact] + public async Task FullyIsolated_OrchestratorIsPinned() + { + var rm = new FakeRepoManager(new() { new() { Id = "repo-1", Name = "Repo" } }); + var svc = CreateDemoService(rm); + var preset = MakePreset(2, WorktreeStrategy.FullyIsolated); + + var group = await svc.CreateGroupFromPresetAsync(preset, + workingDirectory: "/fallback", + repoId: "repo-1"); + + var orchMeta = svc.Organization.Sessions + .First(s => s.GroupId == group!.Id && s.Role == MultiAgentRole.Orchestrator); + Assert.True(orchMeta.IsPinned); + } + + [Fact] + public async Task FullyIsolated_GroupWorktreeIdIsOrchestratorWorktree() + { + var rm = new FakeRepoManager(new() { new() { Id = "repo-1", Name = "Repo" } }); + var svc = CreateDemoService(rm); + var preset = MakePreset(2, WorktreeStrategy.FullyIsolated); + + var group = await svc.CreateGroupFromPresetAsync(preset, + workingDirectory: "/fallback", + repoId: "repo-1"); + + var orchMeta = svc.Organization.Sessions + .First(s => s.GroupId == group!.Id && s.Role == MultiAgentRole.Orchestrator); + + // Group-level WorktreeId should match orchestrator's worktree + Assert.Equal(orchMeta.WorktreeId, group!.WorktreeId); + } + + #endregion + + #region Error Resilience + + [Fact] + public async Task WorktreeCreationFailure_StillCreatesSessions() + { + var rm = new FailingRepoManager(new() { new() { Id = "repo-1", Name = "Repo" } }); + var svc = CreateDemoService(rm); + var preset = MakePreset(3, WorktreeStrategy.FullyIsolated); + + // Should not throw + var group = await svc.CreateGroupFromPresetAsync(preset, + workingDirectory: "/fallback", + repoId: "repo-1"); + + Assert.NotNull(group); + + // Sessions should still be created even though worktrees failed + var members = svc.Organization.Sessions + .Where(s => s.GroupId == group!.Id) + .ToList(); + + Assert.Equal(4, members.Count); // 1 orch + 3 workers + } + + /// + /// A RepoManager that always throws on worktree creation. + /// + private class FailingRepoManager : RepoManager + { + public FailingRepoManager(List repos) + { + 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 = new() }); + 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 git failure"); + } + + public override Task FetchAsync(string repoId, CancellationToken ct = default) + { + throw new InvalidOperationException("Simulated git fetch failure"); + } + } + + #endregion +} diff --git a/PolyPilot/Services/CopilotService.Organization.cs b/PolyPilot/Services/CopilotService.Organization.cs index 7181eb1cbc..914007b0fc 100644 --- a/PolyPilot/Services/CopilotService.Organization.cs +++ b/PolyPilot/Services/CopilotService.Organization.cs @@ -1445,6 +1445,9 @@ public string GetEffectiveModel(string sessionName) } } + var createdWtCount = workerWtIds.Count(id => id != null) + (orchWtId != worktreeId ? 1 : 0); + Debug($"[WorktreeStrategy] Strategy={strategy}, orchDir={orchWorkDir ?? "(null)"}, orchWtId={orchWtId ?? "(none)"}, workerWts created={workerWtIds.Count(id => id != null)}/{preset.WorkerModels.Length}"); + // Create orchestrator session (with uniqueness check matching CreateMultiAgentGroupAsync) var orchName = $"{teamName}-orchestrator"; { int suffix = 1; @@ -1470,6 +1473,7 @@ public string GetEffectiveModel(string sessionName) orchMeta.WorktreeId = orchWtId; // Create worker sessions + Debug($"[WorktreeStrategy] Creating {preset.WorkerModels.Length} workers with strategy={strategy}, repoId={repoId}"); for (int i = 0; i < preset.WorkerModels.Length; i++) { var workerName = $"{teamName}-worker-{i + 1}"; @@ -1479,6 +1483,7 @@ public string GetEffectiveModel(string sessionName) } var workerModel = preset.WorkerModels[i]; var workerWorkDir = workerWorkDirs[i] ?? workingDirectory; + Debug($"[WorktreeStrategy] Worker '{workerName}': wtId={workerWtIds[i] ?? "(none)"}, dir={workerWorkDir ?? "(null)"}"); try { await CreateSessionAsync(workerName, workerModel, workerWorkDir, ct); diff --git a/PolyPilot/Services/RepoManager.cs b/PolyPilot/Services/RepoManager.cs index 5ddbad011d..b0d8e85444 100644 --- a/PolyPilot/Services/RepoManager.cs +++ b/PolyPilot/Services/RepoManager.cs @@ -206,7 +206,7 @@ public async Task AddRepositoryFromLocalAsync(string localPath, /// /// Create a new worktree for a repository on a new branch from origin/main. /// - public async Task CreateWorktreeAsync(string repoId, string branchName, string? baseBranch = null, bool skipFetch = false, CancellationToken ct = default) + public virtual async Task CreateWorktreeAsync(string repoId, string branchName, string? baseBranch = null, bool skipFetch = false, CancellationToken ct = default) { EnsureLoaded(); var repo = _state.Repositories.FirstOrDefault(r => r.Id == repoId) @@ -469,7 +469,7 @@ public void LinkSessionToWorktree(string worktreeId, string sessionName) /// /// Fetch latest from remote for a repository. /// - public async Task FetchAsync(string repoId, CancellationToken ct = default) + public virtual async Task FetchAsync(string repoId, CancellationToken ct = default) { EnsureLoaded(); var repo = _state.Repositories.FirstOrDefault(r => r.Id == repoId) From 6e629f03b5342d007dc323296ddbf1e3c0eef98d Mon Sep 17 00:00:00 2001 From: Shane Date: Wed, 25 Feb 2026 09:19:42 -0600 Subject: [PATCH 14/33] Fix: sanitize team name for git branch names (spaces broke worktree creation) Root cause: 'PR Review Squad' has spaces, which are invalid in git branch names. CreateWorktreeAsync silently failed for all workers because 'git worktree add -b PR Review Squad-worker-1-xxxx' was rejected by git. Only the orchestrator appeared to succeed (fell back to an existing branch). Fix: sanitize team name by replacing non-alphanumeric chars with dashes before using it in branch names. 'PR Review Squad' becomes 'PR-Review-Squad'. Added 2 tests for branch name sanitization (spaces and special chars). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- PolyPilot.Tests/WorktreeStrategyTests.cs | 46 +++++++++++++++++++ .../Services/CopilotService.Organization.cs | 9 ++-- 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/PolyPilot.Tests/WorktreeStrategyTests.cs b/PolyPilot.Tests/WorktreeStrategyTests.cs index 800f5ee303..650955c442 100644 --- a/PolyPilot.Tests/WorktreeStrategyTests.cs +++ b/PolyPilot.Tests/WorktreeStrategyTests.cs @@ -494,4 +494,50 @@ public override Task FetchAsync(string repoId, CancellationToken ct = default) } #endregion + + #region Branch Name Sanitization + + [Fact] + public async Task BranchNames_SpacesReplacedWithDashes() + { + var rm = new FakeRepoManager(new() { new() { Id = "repo-1", Name = "Repo" } }); + var svc = CreateDemoService(rm); + var preset = MakePreset(2, WorktreeStrategy.FullyIsolated); + + // "PR Review Squad" has spaces — branch names must not + await svc.CreateGroupFromPresetAsync(preset, + workingDirectory: "/fallback", + repoId: "repo-1", + nameOverride: "PR Review Squad"); + + // All branch names should have no spaces + Assert.All(rm.CreateCalls, c => + { + Assert.DoesNotContain(" ", c.BranchName); + Assert.StartsWith("PR-Review-Squad-", c.BranchName); + }); + } + + [Fact] + public async Task BranchNames_SpecialCharsRemoved() + { + var rm = new FakeRepoManager(new() { new() { Id = "repo-1", Name = "Repo" } }); + var svc = CreateDemoService(rm); + var preset = MakePreset(1, WorktreeStrategy.FullyIsolated); + + await svc.CreateGroupFromPresetAsync(preset, + workingDirectory: "/fallback", + repoId: "repo-1", + nameOverride: "My Team! @#$%"); + + Assert.All(rm.CreateCalls, c => + { + Assert.DoesNotContain(" ", c.BranchName); + Assert.DoesNotContain("!", c.BranchName); + Assert.DoesNotContain("@", c.BranchName); + Assert.DoesNotContain("#", c.BranchName); + }); + } + + #endregion } diff --git a/PolyPilot/Services/CopilotService.Organization.cs b/PolyPilot/Services/CopilotService.Organization.cs index 914007b0fc..b4f2218475 100644 --- a/PolyPilot/Services/CopilotService.Organization.cs +++ b/PolyPilot/Services/CopilotService.Organization.cs @@ -1379,6 +1379,9 @@ public string GetEffectiveModel(string sessionName) if (group == null) return null; group.WorktreeStrategy = strategy; + // Sanitize team name for use in git branch names (no spaces or special chars) + var branchPrefix = System.Text.RegularExpressions.Regex.Replace(teamName, @"[^a-zA-Z0-9_-]", "-").Trim('-'); + // Store Squad context (routing, decisions) on the group for use during orchestration group.SharedContext = preset.SharedContext; group.RoutingContext = preset.RoutingContext; @@ -1398,7 +1401,7 @@ public string GetEffectiveModel(string sessionName) { try { - var orchWt = await _repoManager.CreateWorktreeAsync(repoId, $"{teamName}-orchestrator-{Guid.NewGuid().ToString()[..4]}", skipFetch: true, ct: ct); + var orchWt = await _repoManager.CreateWorktreeAsync(repoId, $"{branchPrefix}-orchestrator-{Guid.NewGuid().ToString()[..4]}", skipFetch: true, ct: ct); orchWorkDir = orchWt.Path; orchWtId = orchWt.Id; group.WorktreeId = orchWtId; @@ -1418,7 +1421,7 @@ public string GetEffectiveModel(string sessionName) { try { - var wt = await _repoManager.CreateWorktreeAsync(repoId, $"{teamName}-worker-{i + 1}-{Guid.NewGuid().ToString()[..4]}", skipFetch: true, ct: ct); + var wt = await _repoManager.CreateWorktreeAsync(repoId, $"{branchPrefix}-worker-{i + 1}-{Guid.NewGuid().ToString()[..4]}", skipFetch: true, ct: ct); workerWorkDirs[i] = wt.Path; workerWtIds[i] = wt.Id; } @@ -1432,7 +1435,7 @@ public string GetEffectiveModel(string sessionName) { try { - var sharedWorkerWt = await _repoManager.CreateWorktreeAsync(repoId, $"{teamName}-workers-{Guid.NewGuid().ToString()[..4]}", skipFetch: true, ct: ct); + var sharedWorkerWt = await _repoManager.CreateWorktreeAsync(repoId, $"{branchPrefix}-workers-{Guid.NewGuid().ToString()[..4]}", skipFetch: true, ct: ct); for (int i = 0; i < preset.WorkerModels.Length; i++) { workerWorkDirs[i] = sharedWorkerWt.Path; From 45de92658bf08d21566e382a842b574012454036 Mon Sep 17 00:00:00 2001 From: Shane Date: Wed, 25 Feb 2026 09:25:37 -0600 Subject: [PATCH 15/33] Harden worktree cleanup: handle invalid directories and missing repos RemoveWorktreeAsync now wraps Directory.Delete and git worktree prune in try-catch so a failure in either doesn't prevent state cleanup. Also handles the case where the repo is gone but the directory exists. Previously, if git worktree remove failed AND prune threw, the worktree entry was never removed from state and the directory leaked. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- PolyPilot/Services/RepoManager.cs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/PolyPilot/Services/RepoManager.cs b/PolyPilot/Services/RepoManager.cs index b0d8e85444..5e9b93d858 100644 --- a/PolyPilot/Services/RepoManager.cs +++ b/PolyPilot/Services/RepoManager.cs @@ -358,10 +358,15 @@ public async Task RemoveWorktreeAsync(string worktreeId, CancellationToken ct = { // Force cleanup if git worktree remove fails if (Directory.Exists(wt.Path)) - Directory.Delete(wt.Path, recursive: true); - await RunGitAsync(repo.BareClonePath, ct, "worktree", "prune"); + try { Directory.Delete(wt.Path, recursive: true); } catch { } + try { await RunGitAsync(repo.BareClonePath, ct, "worktree", "prune"); } catch { } } } + else if (Directory.Exists(wt.Path)) + { + // No repo found — just delete the directory + try { Directory.Delete(wt.Path, recursive: true); } catch { } + } _state.Worktrees.RemoveAll(w => w.Id == worktreeId); Save(); From ce54980c7a92a21391df935cd142fc3f52f900af Mon Sep 17 00:00:00 2001 From: Shane Date: Wed, 25 Feb 2026 09:32:58 -0600 Subject: [PATCH 16/33] Hide 'Remove Repo' option from multi-agent team menus MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The repo group context menu showed 'Remove Repo' even for multi-agent teams created on that repo. This is confusing and dangerous — removing the repo would orphan the team. Now hidden when group.IsMultiAgent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- PolyPilot/Components/Layout/SessionSidebar.razor | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/PolyPilot/Components/Layout/SessionSidebar.razor b/PolyPilot/Components/Layout/SessionSidebar.razor index f9fa0cbe49..0a1f6cdb5b 100644 --- a/PolyPilot/Components/Layout/SessionSidebar.razor +++ b/PolyPilot/Components/Layout/SessionSidebar.razor @@ -471,9 +471,12 @@ else } } - + @if (!group.IsMultiAgent) + { + + } } else { From 51b2b76a0af2bd4c794ff88a8c17cf7c7373493a Mon Sep 17 00:00:00 2001 From: Shane Date: Wed, 25 Feb 2026 09:46:58 -0600 Subject: [PATCH 17/33] Fix: clean up partial directories when git worktree add fails When git worktree add fails mid-operation, it can leave an empty or partial directory behind. This directory isn't tracked by git's worktree list, so RemoveWorktreeAsync can't clean it up later. Now CreateWorktreeAsync wraps the git call in try-catch and deletes the partial directory before re-throwing, preventing directory leaks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- PolyPilot/Services/RepoManager.cs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/PolyPilot/Services/RepoManager.cs b/PolyPilot/Services/RepoManager.cs index 5e9b93d858..e6df074df1 100644 --- a/PolyPilot/Services/RepoManager.cs +++ b/PolyPilot/Services/RepoManager.cs @@ -224,7 +224,17 @@ public virtual async Task CreateWorktreeAsync(string repoId, strin var worktreeId = Guid.NewGuid().ToString()[..8]; var worktreePath = Path.Combine(WorktreesDir, $"{repoId}-{worktreeId}"); - await RunGitAsync(repo.BareClonePath, ct, "worktree", "add", worktreePath, "-b", branchName, baseRef); + try + { + await RunGitAsync(repo.BareClonePath, ct, "worktree", "add", worktreePath, "-b", branchName, baseRef); + } + catch + { + // git worktree add can leave a partial directory on failure — clean up + if (Directory.Exists(worktreePath)) + try { Directory.Delete(worktreePath, recursive: true); } catch { } + throw; + } var wt = new WorktreeInfo { From ee4eb3723460c701fb4e17df5a1c1334f4aabb58 Mon Sep 17 00:00:00 2001 From: Shane Date: Wed, 25 Feb 2026 09:50:13 -0600 Subject: [PATCH 18/33] Clean up git branches when removing worktrees RemoveWorktreeAsync now deletes the worktree branch after removing the directory. Previously, branches like PR-Review-Squad-worker-1-xxxx accumulated in the bare repo (30+ stale branches found). Also cleaned up 31 stale branches and pruned worktree references. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- PolyPilot/Services/RepoManager.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/PolyPilot/Services/RepoManager.cs b/PolyPilot/Services/RepoManager.cs index e6df074df1..f60fbdee4e 100644 --- a/PolyPilot/Services/RepoManager.cs +++ b/PolyPilot/Services/RepoManager.cs @@ -371,6 +371,9 @@ public async Task RemoveWorktreeAsync(string worktreeId, CancellationToken ct = try { Directory.Delete(wt.Path, recursive: true); } catch { } try { await RunGitAsync(repo.BareClonePath, ct, "worktree", "prune"); } catch { } } + // Clean up the branch too (worktree branches are single-use) + if (!string.IsNullOrEmpty(wt.Branch)) + try { await RunGitAsync(repo.BareClonePath, ct, "branch", "-D", wt.Branch); } catch { } } else if (Directory.Exists(wt.Path)) { From ecdb5e17866be60fa2c60cb7b10b6c123bab9d85 Mon Sep 17 00:00:00 2001 From: Shane Date: Wed, 25 Feb 2026 10:08:52 -0600 Subject: [PATCH 19/33] Track all created worktree IDs on group for reliable cleanup Add CreatedWorktreeIds list to SessionGroup that records every worktree created during squad setup. DeleteGroup now uses this authoritative list for cleanup, preventing leaks when session creation fails after worktree creation succeeds. Adds 4 tests covering tracking across all strategies. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- PolyPilot.Tests/WorktreeStrategyTests.cs | 70 +++++++++++++++++++ PolyPilot/Models/SessionOrganization.cs | 6 ++ .../Services/CopilotService.Organization.cs | 6 ++ 3 files changed, 82 insertions(+) diff --git a/PolyPilot.Tests/WorktreeStrategyTests.cs b/PolyPilot.Tests/WorktreeStrategyTests.cs index 650955c442..362d39b4e5 100644 --- a/PolyPilot.Tests/WorktreeStrategyTests.cs +++ b/PolyPilot.Tests/WorktreeStrategyTests.cs @@ -540,4 +540,74 @@ await svc.CreateGroupFromPresetAsync(preset, } #endregion + + #region CreatedWorktreeIds Tracking + + [Fact] + public async Task FullyIsolated_CreatedWorktreeIds_TracksAllWorktrees() + { + var rm = new FakeRepoManager(new() { new() { Id = "repo-1", Name = "Repo" } }); + var svc = CreateDemoService(rm); + var preset = MakePreset(3, WorktreeStrategy.FullyIsolated); + + var group = await svc.CreateGroupFromPresetAsync(preset, + workingDirectory: "/fallback", + repoId: "repo-1"); + + // 1 orchestrator + 3 workers = 4 worktrees + Assert.Equal(4, group!.CreatedWorktreeIds.Count); + Assert.Equal(4, group.CreatedWorktreeIds.Distinct().Count()); + } + + [Fact] + public async Task OrchestratorIsolated_CreatedWorktreeIds_TracksAllWorktrees() + { + var rm = new FakeRepoManager(new() { new() { Id = "repo-1", Name = "Repo" } }); + var svc = CreateDemoService(rm); + var preset = MakePreset(3, WorktreeStrategy.OrchestratorIsolated); + + var group = await svc.CreateGroupFromPresetAsync(preset, + workingDirectory: "/fallback", + repoId: "repo-1"); + + // 1 orchestrator + 1 shared worker = 2 worktrees + Assert.Equal(2, group!.CreatedWorktreeIds.Count); + Assert.Equal(2, group.CreatedWorktreeIds.Distinct().Count()); + } + + [Fact] + public async Task Shared_CreatedWorktreeIds_Empty() + { + var rm = new FakeRepoManager(new() { new() { Id = "repo-1", Name = "Repo" } }); + var svc = CreateDemoService(rm); + var preset = MakePreset(3, WorktreeStrategy.Shared); + + var group = await svc.CreateGroupFromPresetAsync(preset, + workingDirectory: "/fallback", + repoId: "repo-1"); + + Assert.Empty(group!.CreatedWorktreeIds); + } + + [Fact] + public async Task FullyIsolated_CreatedWorktreeIds_MatchesSessionWorktreeIds() + { + var rm = new FakeRepoManager(new() { new() { Id = "repo-1", Name = "Repo" } }); + var svc = CreateDemoService(rm); + var preset = MakePreset(2, WorktreeStrategy.FullyIsolated); + + var group = await svc.CreateGroupFromPresetAsync(preset, + workingDirectory: "/fallback", + repoId: "repo-1"); + + var sessionWtIds = svc.Organization.Sessions + .Where(s => s.GroupId == group!.Id && s.WorktreeId != null) + .Select(s => s.WorktreeId!) + .ToHashSet(); + + // All session worktree IDs should be in CreatedWorktreeIds + Assert.All(sessionWtIds, id => Assert.Contains(id, group!.CreatedWorktreeIds)); + } + + #endregion } diff --git a/PolyPilot/Models/SessionOrganization.cs b/PolyPilot/Models/SessionOrganization.cs index 59328af99d..03903c7fd5 100644 --- a/PolyPilot/Models/SessionOrganization.cs +++ b/PolyPilot/Models/SessionOrganization.cs @@ -38,6 +38,12 @@ public class SessionGroup /// How worktrees are allocated across sessions in this group. public WorktreeStrategy WorktreeStrategy { get; set; } = WorktreeStrategy.Shared; + /// + /// All worktree IDs created for this group (orchestrator + workers). + /// Used by DeleteGroup for reliable cleanup even when session creation fails. + /// + public List CreatedWorktreeIds { get; set; } = new(); + /// Active reflection state for OrchestratorReflect mode. Null when not in a reflect loop. public ReflectionCycle? ReflectionState { get; set; } diff --git a/PolyPilot/Services/CopilotService.Organization.cs b/PolyPilot/Services/CopilotService.Organization.cs index b4f2218475..54842f1235 100644 --- a/PolyPilot/Services/CopilotService.Organization.cs +++ b/PolyPilot/Services/CopilotService.Organization.cs @@ -419,6 +419,9 @@ public void DeleteGroup(string groupId) // Collect all worktree IDs for cleanup before removing metadata var worktreeIds = new HashSet(); if (group?.WorktreeId != null) worktreeIds.Add(group.WorktreeId); + // CreatedWorktreeIds is the authoritative list (covers cases where session creation failed) + if (group?.CreatedWorktreeIds != null) + foreach (var id in group.CreatedWorktreeIds) worktreeIds.Add(id); foreach (var m in Organization.Sessions.Where(m => m.GroupId == groupId)) if (m.WorktreeId != null) worktreeIds.Add(m.WorktreeId); @@ -1405,6 +1408,7 @@ public string GetEffectiveModel(string sessionName) orchWorkDir = orchWt.Path; orchWtId = orchWt.Id; group.WorktreeId = orchWtId; + group.CreatedWorktreeIds.Add(orchWtId); } catch (Exception ex) { @@ -1424,6 +1428,7 @@ public string GetEffectiveModel(string sessionName) var wt = await _repoManager.CreateWorktreeAsync(repoId, $"{branchPrefix}-worker-{i + 1}-{Guid.NewGuid().ToString()[..4]}", skipFetch: true, ct: ct); workerWorkDirs[i] = wt.Path; workerWtIds[i] = wt.Id; + group.CreatedWorktreeIds.Add(wt.Id); } catch (Exception ex) { @@ -1436,6 +1441,7 @@ public string GetEffectiveModel(string sessionName) try { var sharedWorkerWt = await _repoManager.CreateWorktreeAsync(repoId, $"{branchPrefix}-workers-{Guid.NewGuid().ToString()[..4]}", skipFetch: true, ct: ct); + group.CreatedWorktreeIds.Add(sharedWorkerWt.Id); for (int i = 0; i < preset.WorkerModels.Length; i++) { workerWorkDirs[i] = sharedWorkerWt.Path; From 12a682151debc316ec133abc87b3bc88c1c7db5f Mon Sep 17 00:00:00 2001 From: Shane Date: Wed, 25 Feb 2026 10:35:09 -0600 Subject: [PATCH 20/33] Redesign New Session form: repo-first flow with mode chips and worktree strategy Replace the hidden nested worktree picker with a flat, repo-first design matching the multi-agent squad creation flow: - Mode chips: Repo | Directory | Empty (Repo default when repos exist) - Worktree segment: Auto | Branch | PR | Existing - Auto mode generates branch name from session name (zero-config) - Auto-naming: session name fills from branch/PR input - Removed Options > Select worktree nesting (now all visible by default) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Components/Layout/CreateSessionForm.razor | 451 ++++++++---------- .../Layout/CreateSessionForm.razor.css | 378 +++++---------- .../Components/Layout/SessionSidebar.razor | 10 +- 3 files changed, 340 insertions(+), 499 deletions(-) diff --git a/PolyPilot/Components/Layout/CreateSessionForm.razor b/PolyPilot/Components/Layout/CreateSessionForm.razor index 3842092814..058e9ff14f 100644 --- a/PolyPilot/Components/Layout/CreateSessionForm.razor +++ b/PolyPilot/Components/Layout/CreateSessionForm.razor @@ -11,117 +11,94 @@ else {
- - + @* Mode chips — shown only when repos exist *@ + @if (RepoManager.Repositories.Count > 0) + { +
+ + + +
+ } - @if (showOptions) + @* Repo mode: repo selector + worktree strategy *@ + @if (mode == FormMode.Repo) { -
- @if (RepoManager.Repositories.Count > 0) + @if (RepoManager.Repositories.Count > 1) + { + + } + else + { +
📦 @RepoManager.Repositories[0].Name
+ } + +
+ + + + +
+ + @if (wtMode == WtMode.Branch) + { + + } + else if (wtMode == WtMode.PR) + { + + } + else if (wtMode == WtMode.Existing) + { + var repoWts = RepoManager.Worktrees.Where(w => w.RepoId == selectedRepoId).ToList(); + @if (repoWts.Count > 0) { -
- - @if (selectedWorktreeBranch != null) - { -
- ⑂ @selectedWorktreeBranch - -
- } - else + - } - else - { - - } - -
- @if (!string.IsNullOrEmpty(worktreeError)) - { -
⚠ @worktreeError
- } - } - else - { - - } - } -
- } -
+ } - @if (filterRepoId == null) + else { -
- -
- - -
-
+
No existing worktrees — try ⚡ Auto or ⑂ Branch
} + } + + @if (!string.IsNullOrEmpty(worktreeError)) + { +
⚠ @worktreeError
+ } + } + else if (mode == FormMode.Directory) + { +
+ +
} + + + @@ -140,214 +117,196 @@ else [Parameter] public string SelectedModel { get; set; } = "claude-opus-4.6"; [Parameter] public EventCallback SelectedModelChanged { get; set; } - [Parameter] public EventCallback<(string Name, string Model, string Directory, string? WorktreeId, string? InitialPrompt)> OnCreate { get; set; } + [Parameter] public EventCallback<(string Name, string Model, string Directory, string? WorktreeId, string? InitialPrompt, string? RepoId, string? BranchName, int? PrNumber)> OnCreate { get; set; } [Parameter] public EventCallback OnBrowseDirectory { get; set; } [Parameter] public string SessionName { get; set; } = ""; [Parameter] public EventCallback SessionNameChanged { get; set; } - private bool isExpanded = false; - private bool showOptions = false; - private bool showWorktreePicker = false; - private string? filterRepoId; // Filter worktree picker to a specific repo + private enum FormMode { Repo, Directory, Empty } + private enum WtMode { Auto, Branch, PR, Existing } + + private bool isExpanded; + private FormMode mode; + private WtMode wtMode = WtMode.Auto; + private string? selectedRepoId; + private string? selectedWorktreeId; + private string branchInput = ""; + private string prInput = ""; private string initialPrompt = ""; private string sessionDirectory = ""; - private string? selectedWorktreePath; - private string? selectedWorktreeId; - private string? selectedWorktreeBranch; - - private string? newWorktreeRepoId; - private string newWorktreeBranch = ""; - private string newWorktreePr = ""; - private string newWorktreeMode = "branch"; private string? worktreeError; private bool isCreatingWorktree; + private bool nameManuallyEdited; private void Expand() { isExpanded = true; - filterRepoId = null; // Clear any repo filter when manually expanding + nameManuallyEdited = false; + mode = RepoManager.Repositories.Count > 0 ? FormMode.Repo : FormMode.Empty; + if (RepoManager.Repositories.Count > 0) + selectedRepoId = RepoManager.Repositories[0].Id; + AutoSelectFirstWorktree(); } + public void ExpandForRepo(string repoId) { isExpanded = true; - showOptions = true; - worktreeError = null; - filterRepoId = repoId; // Filter to this repo only - showWorktreePicker = true; - newWorktreeRepoId = null; // Clear any open form so the button shows - - // If worktrees exist for this repo, auto-select the first one but keep picker open - var existingWorktrees = RepoManager.Worktrees.Where(w => w.RepoId == repoId).ToList(); - if (existingWorktrees.Count > 0) - { - var wt = existingWorktrees[0]; - sessionDirectory = wt.Path; - selectedWorktreePath = wt.Path; - selectedWorktreeId = wt.Id; - selectedWorktreeBranch = wt.Branch; - // Don't close the picker - we want to show the "+ New worktree..." button - if (string.IsNullOrWhiteSpace(SessionName)) - { - SessionName = wt.Branch; - _ = SessionNameChanged.InvokeAsync(SessionName); - } - } - + nameManuallyEdited = false; + mode = FormMode.Repo; + selectedRepoId = repoId; + wtMode = WtMode.Auto; + AutoSelectFirstWorktree(); StateHasChanged(); } + private void Collapse() { isExpanded = false; - showOptions = false; - showWorktreePicker = false; - newWorktreeRepoId = null; - filterRepoId = null; // Clear repo filter + worktreeError = null; + branchInput = ""; + prInput = ""; + sessionDirectory = ""; + initialPrompt = ""; + selectedWorktreeId = null; + nameManuallyEdited = false; } - private void SetWorktreeMode(string mode) => newWorktreeMode = mode; - private void SetModeBranch() => newWorktreeMode = "branch"; - private void SetModePr() => newWorktreeMode = "pr"; + private void SetMode(FormMode newMode) + { + mode = newMode; + worktreeError = null; + if (!nameManuallyEdited) { SessionName = ""; _ = SessionNameChanged.InvokeAsync(""); } + } - private void ToggleWorktreePicker() + private void SetWtMode(WtMode newWtMode) { - showWorktreePicker = !showWorktreePicker; - // When manually toggling picker, clear the filter to show all repos - if (showWorktreePicker) filterRepoId = null; + wtMode = newWtMode; + worktreeError = null; + branchInput = ""; + prInput = ""; + if (newWtMode == WtMode.Existing) + AutoSelectFirstWorktree(); + if (!nameManuallyEdited) UpdateAutoName(); } - private async Task OnNameInput(ChangeEventArgs e) + private void OnRepoChanged(ChangeEventArgs e) { - SessionName = e.Value?.ToString() ?? ""; - await SessionNameChanged.InvokeAsync(SessionName); + selectedRepoId = e.Value?.ToString(); + if (wtMode == WtMode.Existing) AutoSelectFirstWorktree(); + if (!nameManuallyEdited) UpdateAutoName(); } - private async Task OnModelSelected(string model) + private void OnExistingWtChanged(ChangeEventArgs e) { - SelectedModel = model; - await SelectedModelChanged.InvokeAsync(model); + selectedWorktreeId = e.Value?.ToString(); + if (!nameManuallyEdited) UpdateAutoName(); + } + + private void AutoSelectFirstWorktree() + { + if (selectedRepoId == null) return; + var first = RepoManager.Worktrees.FirstOrDefault(w => w.RepoId == selectedRepoId); + selectedWorktreeId = first?.Id; } - private void SelectWorktree(WorktreeInfo wt) + private void UpdateAutoName() { - sessionDirectory = wt.Path; - selectedWorktreePath = wt.Path; - selectedWorktreeId = wt.Id; - selectedWorktreeBranch = wt.Branch; - showWorktreePicker = false; - newWorktreeRepoId = null; - - if (string.IsNullOrWhiteSpace(SessionName)) + if (nameManuallyEdited) return; + string? auto = null; + if (mode == FormMode.Repo) { - SessionName = wt.Branch; - _ = SessionNameChanged.InvokeAsync(SessionName); + if (wtMode == WtMode.Branch && !string.IsNullOrWhiteSpace(branchInput)) + auto = branchInput.Trim(); + else if (wtMode == WtMode.PR && !string.IsNullOrWhiteSpace(prInput)) + auto = $"PR #{prInput.Trim().TrimStart('#')}"; + else if (wtMode == WtMode.Existing && selectedWorktreeId != null) + { + var wt = RepoManager.Worktrees.FirstOrDefault(w => w.Id == selectedWorktreeId); + if (wt != null) auto = wt.Branch; + } + } + if (auto != null) + { + SessionName = auto; + _ = SessionNameChanged.InvokeAsync(auto); } } - private void ClearWorktree() + private async Task OnNameInput(ChangeEventArgs e) { - selectedWorktreePath = null; - selectedWorktreeId = null; - selectedWorktreeBranch = null; - sessionDirectory = ""; + SessionName = e.Value?.ToString() ?? ""; + nameManuallyEdited = !string.IsNullOrWhiteSpace(SessionName); + await SessionNameChanged.InvokeAsync(SessionName); } - private void StartNewWorktreeForm(string repoId) + private async Task OnModelSelected(string model) { - newWorktreeRepoId = repoId; - newWorktreeBranch = ""; - newWorktreePr = ""; - newWorktreeMode = "branch"; - worktreeError = null; + SelectedModel = model; + await SelectedModelChanged.InvokeAsync(model); } - private async Task HandleNewWorktreeKeyUp(KeyboardEventArgs e) + private async Task HandleInputKeyUp(KeyboardEventArgs e) { - if (e.Key == "Enter") await CreateAndSelectWorktree(); - else if (e.Key == "Escape") newWorktreeRepoId = null; + if (!nameManuallyEdited) UpdateAutoName(); + if (e.Key == "Enter") await TriggerCreate(); + else if (e.Key == "Escape") Collapse(); } - private async Task CreateAndSelectWorktree() + private async Task HandleKeyUp(KeyboardEventArgs e) { - if (isCreatingWorktree || newWorktreeRepoId == null) return; - if (newWorktreeMode == "pr") + if (e.Key == "Escape") Collapse(); + else if (e.Key == "Enter") await TriggerCreate(); + } + + private async Task TriggerCreate() + { + if (string.IsNullOrWhiteSpace(SessionName) || IsCreating || isCreatingWorktree) return; + + var prompt = string.IsNullOrWhiteSpace(initialPrompt) ? null : initialPrompt.Trim(); + string? repoId = null; + string? branchName = null; + int? prNumber = null; + string? wtId = null; + string dir = ""; + + if (mode == FormMode.Repo && selectedRepoId != null) { - if (!int.TryParse(newWorktreePr.Trim().TrimStart('#'), out var prNum) || prNum <= 0) - { worktreeError = "Enter a valid PR number"; return; } - isCreatingWorktree = true; worktreeError = null; - try + repoId = selectedRepoId; + if (wtMode == WtMode.Auto) { - if (CopilotService.IsRemoteMode) - { - var result = await CopilotService.CreateWorktreeViaBridgeAsync(newWorktreeRepoId, null, prNum); - var wt = new WorktreeInfo { Id = result.WorktreeId, RepoId = result.RepoId, Branch = result.Branch, Path = result.Path, PrNumber = result.PrNumber }; - RepoManager.AddRemoteWorktree(wt); - SelectWorktree(wt); - } - else - { - var wt = await RepoManager.CreateWorktreeFromPrAsync(newWorktreeRepoId, prNum); - SelectWorktree(wt); - } - if (string.IsNullOrWhiteSpace(SessionName)) - { SessionName = $"PR #{prNum}"; await SessionNameChanged.InvokeAsync(SessionName); } + // Auto-generate branch name from session name + var sanitized = System.Text.RegularExpressions.Regex.Replace(SessionName.Trim(), @"[^a-zA-Z0-9_-]", "-").Trim('-'); + branchName = $"{sanitized}-{Guid.NewGuid().ToString()[..4]}"; } - catch (Exception ex) { worktreeError = ex.Message; } - finally { isCreatingWorktree = false; } - } - else - { - if (string.IsNullOrWhiteSpace(newWorktreeBranch)) return; - isCreatingWorktree = true; worktreeError = null; - try + else if (wtMode == WtMode.Branch) { - WorktreeInfo wt; - if (CopilotService.IsRemoteMode) - { - var result = await CopilotService.CreateWorktreeViaBridgeAsync(newWorktreeRepoId, newWorktreeBranch.Trim(), null); - wt = new WorktreeInfo { Id = result.WorktreeId, RepoId = result.RepoId, Branch = result.Branch, Path = result.Path, PrNumber = result.PrNumber }; - RepoManager.AddRemoteWorktree(wt); - } - else - { - wt = await RepoManager.CreateWorktreeAsync(newWorktreeRepoId, newWorktreeBranch.Trim()); - } - SelectWorktree(wt); - if (string.IsNullOrWhiteSpace(SessionName)) - { SessionName = wt.Branch; await SessionNameChanged.InvokeAsync(SessionName); } + if (string.IsNullOrWhiteSpace(branchInput)) { worktreeError = "Enter a branch name"; return; } + branchName = branchInput.Trim(); + } + else if (wtMode == WtMode.PR) + { + if (!int.TryParse(prInput.Trim().TrimStart('#'), out var pr) || pr <= 0) + { worktreeError = "Enter a valid PR number"; return; } + prNumber = pr; + } + else if (wtMode == WtMode.Existing) + { + if (string.IsNullOrEmpty(selectedWorktreeId)) { worktreeError = "Select a worktree"; return; } + wtId = selectedWorktreeId; } - catch (Exception ex) { worktreeError = ex.Message; } - finally { isCreatingWorktree = false; } } - } - - private async Task RemoveWorktree(string worktreeId) - { - try + else if (mode == FormMode.Directory) { - if (selectedWorktreeId == worktreeId) ClearWorktree(); - if (CopilotService.IsRemoteMode) - await CopilotService.RemoveWorktreeViaBridgeAsync(worktreeId); - else - await RepoManager.RemoveWorktreeAsync(worktreeId); + dir = sessionDirectory; } - catch (Exception ex) { worktreeError = ex.Message; } - } - private async Task TriggerCreate() - { - if (!string.IsNullOrWhiteSpace(SessionName)) - { - var prompt = string.IsNullOrWhiteSpace(initialPrompt) ? null : initialPrompt.Trim(); - await OnCreate.InvokeAsync((SessionName, SelectedModel, sessionDirectory, selectedWorktreeId, prompt)); - selectedWorktreePath = null; selectedWorktreeId = null; selectedWorktreeBranch = null; - sessionDirectory = ""; initialPrompt = ""; - isExpanded = false; showOptions = false; - } - } + worktreeError = null; + await OnCreate.InvokeAsync((SessionName, SelectedModel, dir, wtId, prompt, repoId, branchName, prNumber)); - private async Task HandleKeyUp(KeyboardEventArgs e) - { - if (e.Key == "Escape") Collapse(); + // Reset form + branchInput = ""; prInput = ""; sessionDirectory = ""; initialPrompt = ""; + selectedWorktreeId = null; nameManuallyEdited = false; + isExpanded = false; } private async Task BrowseDirectory() @@ -357,7 +316,7 @@ else { var dir = await FolderPickerService.PickFolderAsync(); if (!string.IsNullOrEmpty(dir)) - { sessionDirectory = dir; selectedWorktreePath = null; selectedWorktreeId = null; selectedWorktreeBranch = null; StateHasChanged(); } + { sessionDirectory = dir; StateHasChanged(); } } catch (Exception ex) { Console.WriteLine($"Folder picker error: {ex.Message}"); } #else @@ -367,7 +326,7 @@ else public void SetDirectory(string path) { - sessionDirectory = path; selectedWorktreePath = null; selectedWorktreeId = null; selectedWorktreeBranch = null; + sessionDirectory = path; StateHasChanged(); } } diff --git a/PolyPilot/Components/Layout/CreateSessionForm.razor.css b/PolyPilot/Components/Layout/CreateSessionForm.razor.css index 43f5c06068..46b7229410 100644 --- a/PolyPilot/Components/Layout/CreateSessionForm.razor.css +++ b/PolyPilot/Components/Layout/CreateSessionForm.razor.css @@ -21,152 +21,192 @@ .new-session-form { display: flex; flex-direction: column; - gap: 0.6rem; + gap: 0.5rem; padding: 0.75rem; border: none; border-radius: 10px; background: var(--surface-secondary); } -.ns-name { - width: 100%; - padding: 0.5rem 0.65rem; - border: 1px solid transparent; +/* === Mode Chips === */ +.ns-mode-chips { + display: flex; + gap: 0.25rem; + padding-bottom: 0.15rem; +} + +.ns-chip { + flex: 1; + padding: 0.3rem 0.4rem; + border: 1px solid var(--control-border); border-radius: 6px; - background: var(--control-bg); - color: var(--text-primary); - font-size: var(--type-body); - box-sizing: border-box; - transition: border-color 0.15s; + background: none; + color: var(--text-dim); + cursor: pointer; + font-size: var(--type-caption1); + font-weight: 500; + text-align: center; + transition: all 0.15s; + white-space: nowrap; } -.ns-name:focus { - outline: none; - border-color: var(--accent-primary); +.ns-chip:hover { + color: var(--text-primary); + background: var(--hover-bg); } -.ns-name::placeholder { - color: var(--text-dim); +.ns-chip.active { + background: var(--accent-primary); + color: #fff; + border-color: var(--accent-primary); } -.ns-prompt { +/* === Repo selector === */ +.ns-repo-select { width: 100%; - padding: 0.45rem 0.65rem; - border: 1px solid transparent; + padding: 0.35rem 0.5rem; + border: 1px solid var(--control-border); border-radius: 6px; background: var(--control-bg); color: var(--text-primary); font-size: var(--type-footnote); - font-family: inherit; - resize: vertical; - min-height: 2.2rem; - max-height: 6rem; - box-sizing: border-box; - transition: border-color 0.15s; + cursor: pointer; } -.ns-prompt:focus { +.ns-repo-select:focus { outline: none; border-color: var(--accent-primary); } -.ns-prompt::placeholder { - color: var(--text-dim); -} - -/* === Options Panel === */ -.ns-options { - display: flex; - flex-direction: column; - gap: 0.5rem; - padding-top: 0.25rem; - border-top: 1px solid var(--control-border); +.ns-repo-label { + font-size: var(--type-footnote); + color: var(--text-secondary); + padding: 0.15rem 0; } -.ns-option-group { +/* === Worktree Segment Control === */ +.ns-wt-segment { display: flex; - flex-direction: column; - gap: 0.25rem; + border: 1px solid var(--control-border); + border-radius: 6px; + overflow: hidden; } -.ns-label { - font-size: var(--type-caption2); +.ns-seg { + flex: 1; + padding: 0.3rem 0.2rem; + border: none; + border-right: 1px solid var(--control-border); + background: none; color: var(--text-dim); - font-weight: 600; - text-transform: uppercase; - letter-spacing: 0.03em; + cursor: pointer; + font-size: var(--type-caption2); + font-weight: 500; + text-align: center; + transition: all 0.15s; + white-space: nowrap; } -.ns-option-btn { - background: none; - border: none; - color: var(--text-secondary); - cursor: pointer; - font-size: var(--type-footnote); - text-align: left; - padding: 0.2rem 0; +.ns-seg:last-child { + border-right: none; } -.ns-option-btn:hover { +.ns-seg:hover { color: var(--text-primary); + background: var(--hover-bg); } -.ns-wt-badge { - display: flex; - align-items: center; - gap: 0.3rem; +.ns-seg.active { + background: var(--accent-primary); + color: #fff; +} + +/* === Shared input style === */ +.ns-input { + width: 100%; + padding: 0.4rem 0.55rem; + border: 1px solid var(--control-border); + border-radius: 6px; + background: var(--control-bg); + color: var(--text-primary); font-size: var(--type-footnote); - color: var(--accent-primary); + box-sizing: border-box; + transition: border-color 0.15s; } -.ns-badge-clear { - background: none; - border: none; +.ns-input:focus { + outline: none; + border-color: var(--accent-primary); +} + +.ns-input::placeholder { color: var(--text-dim); - cursor: pointer; +} + +.ns-hint { font-size: var(--type-caption2); - padding: 0 0.2rem; + color: var(--text-dim); + padding: 0.2rem 0; + font-style: italic; } -.ns-badge-clear:hover { +.ns-name { + width: 100%; + padding: 0.5rem 0.65rem; + border: 1px solid transparent; + border-radius: 6px; + background: var(--control-bg); color: var(--text-primary); + font-size: var(--type-body); + box-sizing: border-box; + transition: border-color 0.15s; } -.ns-wt-picker { - background: var(--control-bg); - border: 1px solid var(--control-border); - border-radius: 6px; - overflow: hidden; - max-height: 200px; - overflow-y: auto; +.ns-name:focus { + outline: none; + border-color: var(--accent-primary); } -.ns-dir-row { - display: flex; - gap: 0.3rem; +.ns-name::placeholder { + color: var(--text-dim); } -.ns-dir-input { - flex: 1; - min-width: 0; - padding: 0.3rem 0.5rem; - border: 1px solid var(--control-border); - border-radius: 4px; +.ns-prompt { + width: 100%; + padding: 0.45rem 0.65rem; + border: 1px solid transparent; + border-radius: 6px; background: var(--control-bg); color: var(--text-primary); font-size: var(--type-footnote); + font-family: inherit; + resize: vertical; + min-height: 2.2rem; + max-height: 6rem; + box-sizing: border-box; + transition: border-color 0.15s; } -.ns-dir-input:focus { +.ns-prompt:focus { outline: none; border-color: var(--accent-primary); } +.ns-prompt::placeholder { + color: var(--text-dim); +} + +/* === Directory row === */ +.ns-dir-row { + display: flex; + gap: 0.3rem; +} + .ns-dir-browse { - padding: 0.3rem 0.5rem; + padding: 0.35rem 0.5rem; background: var(--control-bg); border: 1px solid var(--control-border); - border-radius: 4px; + border-radius: 6px; color: var(--text-primary); cursor: pointer; font-size: var(--type-footnote); @@ -190,26 +230,10 @@ min-width: 0; } -.ns-options-toggle { - background: none; - border: none; - color: var(--text-dim); - cursor: pointer; - font-size: var(--type-caption1); - padding: 0.25rem 0.35rem; - white-space: nowrap; - border-radius: 4px; - transition: all 0.15s; -} - -.ns-options-toggle:hover { - color: var(--text-primary); - background: var(--hover-bg); -} - .ns-footer-actions { display: flex; gap: 0.35rem; + margin-left: auto; } .ns-cancel { @@ -256,155 +280,6 @@ padding: 0.1rem 0; } -/* === Worktree picker items (reused from before) === */ -.wt-repo-name { - font-size: var(--type-caption2); - color: var(--text-dim); - padding: 0.3rem 0.5rem 0.1rem; - font-weight: 600; - text-transform: uppercase; - letter-spacing: 0.03em; -} - -.wt-option-row { - display: flex; - align-items: center; -} - -.wt-option-row .wt-option { - flex: 1; - min-width: 0; -} - -.wt-option { - display: flex; - align-items: center; - gap: 0.5rem; - width: 100%; - padding: 0.3rem 0.5rem; - border: none; - background: none; - color: var(--text-primary); - cursor: pointer; - font-size: var(--type-footnote); - text-align: left; -} - -.wt-option:hover { - background: var(--hover-bg); -} - -.wt-option.selected { - background: var(--hover-bg); -} - -.wt-option.wt-new { - color: var(--accent-primary); - font-size: var(--type-footnote); -} - -.wt-delete-btn { - background: none; - border: none; - color: var(--text-dim); - cursor: pointer; - font-size: var(--type-footnote); - padding: 0.2rem 0.4rem; - opacity: 0; - transition: opacity 0.15s; -} - -.wt-option-row:hover .wt-delete-btn { - opacity: 1; -} - -.wt-delete-btn:hover { - color: #ff6b6b; -} - -.wt-pr-badge { - font-size: var(--type-caption2); - color: var(--accent-primary); - margin-left: auto; -} - -.wt-mode-toggle { - display: flex; - padding: 0.2rem 0.5rem; - gap: 0; -} - -.wt-mode { - flex: 1; - padding: 0.2rem 0; - border: 1px solid var(--control-border); - background: none; - color: var(--text-dim); - cursor: pointer; - font-size: var(--type-caption2); - font-weight: 600; - text-align: center; -} - -.wt-mode:first-child { - border-radius: 4px 0 0 4px; -} - -.wt-mode:last-child { - border-radius: 0 4px 4px 0; - border-left: none; -} - -.wt-mode.active { - background: var(--control-bg); - color: var(--text-primary); - border-color: var(--accent-primary); -} - -.wt-new-form { - display: flex; - gap: 0.3rem; - padding: 0.25rem 0.5rem; -} - -.wt-branch-input { - flex: 1; - min-width: 0; - padding: 0.25rem 0.4rem; - border: 1px solid var(--control-border); - border-radius: 4px; - background: var(--surface-secondary); - color: var(--text-primary); - font-size: var(--type-footnote); -} - -.wt-branch-input:focus { - outline: none; - border-color: var(--accent-primary); -} - -.wt-create-btn { - padding: 0.25rem 0.5rem; - border: none; - border-radius: 4px; - background: var(--accent-primary); - color: var(--text-primary); - cursor: pointer; - font-size: var(--type-footnote); - font-weight: bold; -} - -.wt-create-btn:disabled { - opacity: 0.5; - cursor: not-allowed; -} - -.wt-error { - color: #ff6b6b; - font-size: var(--type-caption2); - padding: 0.15rem 0.5rem; -} - /* === Mobile === */ @media (max-width: 640px) { .new-session-btn { @@ -420,4 +295,9 @@ .ns-name { font-size: var(--type-callout); } + + .ns-chip { + font-size: var(--type-caption2); + padding: 0.25rem 0.3rem; + } } diff --git a/PolyPilot/Components/Layout/SessionSidebar.razor b/PolyPilot/Components/Layout/SessionSidebar.razor index 0a1f6cdb5b..7e4f8a5c2c 100644 --- a/PolyPilot/Components/Layout/SessionSidebar.razor +++ b/PolyPilot/Components/Layout/SessionSidebar.razor @@ -932,7 +932,7 @@ else } } - private async Task HandleCreateSession((string Name, string Model, string Directory, string? WorktreeId, string? InitialPrompt) args) + private async Task HandleCreateSession((string Name, string Model, string Directory, string? WorktreeId, string? InitialPrompt, string? RepoId, string? BranchName, int? PrNumber) args) { if (isCreating) return; isCreating = true; @@ -941,11 +941,13 @@ else { AgentSessionInfo sessionInfo; - if (!string.IsNullOrEmpty(args.WorktreeId)) + if (!string.IsNullOrEmpty(args.RepoId) || !string.IsNullOrEmpty(args.WorktreeId)) { - // Atomic worktree+session creation + // Repo-based: atomic worktree+session creation sessionInfo = await CopilotService.CreateSessionWithWorktreeAsync( - repoId: null!, // Not needed when worktreeId is provided + repoId: args.RepoId ?? null!, + branchName: args.BranchName, + prNumber: args.PrNumber, worktreeId: args.WorktreeId, sessionName: args.Name, model: args.Model, From 753c5dd3757af9fc18efe7e64fbf42e15bc12869 Mon Sep 17 00:00:00 2001 From: Shane Date: Wed, 25 Feb 2026 10:47:35 -0600 Subject: [PATCH 21/33] Unify session close into single menu item with delete worktree checkbox MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the two separate 'Close Session' and 'Close & Delete Worktree' menu items with a single '🗑 Close Session…' item that shows an inline confirmation panel. When the session has an associated worktree, a checkbox appears to optionally delete the worktree and branch too. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Components/Layout/SessionListItem.razor | 49 +++++++++++-- .../Layout/SessionListItem.razor.css | 72 +++++++++++++++++++ 2 files changed, 115 insertions(+), 6 deletions(-) diff --git a/PolyPilot/Components/Layout/SessionListItem.razor b/PolyPilot/Components/Layout/SessionListItem.razor index f032a86d42..0f0c06e530 100644 --- a/PolyPilot/Components/Layout/SessionListItem.razor +++ b/PolyPilot/Components/Layout/SessionListItem.razor @@ -197,14 +197,26 @@ } - - @if (!string.IsNullOrEmpty(Meta?.WorktreeId)) + @if (showCloseConfirm) { - +
+
Close "@Session.Name"?
+ @if (!string.IsNullOrEmpty(Meta?.WorktreeId)) + { + var wt = RepoManager.Worktrees.FirstOrDefault(w => w.Id == Meta.WorktreeId); + + } +
+ + +
+
}
} @@ -234,6 +246,31 @@ [Parameter] public EventCallback OnReportBug { get; set; } [Parameter] public EventCallback OnFixWithCopilot { get; set; } + private bool showCloseConfirm; + private bool deleteWorktree; + + private void ShowCloseConfirm() + { + showCloseConfirm = true; + deleteWorktree = false; + } + + private void HideCloseConfirm() + { + showCloseConfirm = false; + _ = OnCloseMenu.InvokeAsync(); + } + + private async Task ConfirmClose() + { + showCloseConfirm = false; + await OnCloseMenu.InvokeAsync(); + if (deleteWorktree && !string.IsNullOrEmpty(Meta?.WorktreeId)) + await OnCloseAndDeleteWorktree.InvokeAsync(); + else + await OnClose.InvokeAsync(); + } + private async Task HandleRenameKeyUp(KeyboardEventArgs e) { if (e.Key == "Enter") await OnCommitRename.InvokeAsync(); diff --git a/PolyPilot/Components/Layout/SessionListItem.razor.css b/PolyPilot/Components/Layout/SessionListItem.razor.css index 6a76ddf420..7a7970c30f 100644 --- a/PolyPilot/Components/Layout/SessionListItem.razor.css +++ b/PolyPilot/Components/Layout/SessionListItem.razor.css @@ -319,3 +319,75 @@ padding: 0.15rem 0; opacity: 0.8; } + +/* === Close confirmation panel === */ +.close-confirm { + display: flex; + flex-direction: column; + gap: 0.4rem; + padding: 0.6rem 0.65rem; + background: var(--surface-secondary); + border: 1px solid var(--control-border); + border-radius: 8px; + margin-top: 0.3rem; +} + +.close-confirm-title { + font-size: var(--type-footnote); + font-weight: 600; + color: var(--text-primary); +} + +.close-confirm-option { + display: flex; + align-items: center; + gap: 0.4rem; + font-size: var(--type-footnote); + color: var(--text-secondary); + cursor: pointer; + padding: 0.15rem 0; +} + +.close-confirm-option input[type="checkbox"] { + accent-color: var(--accent-primary); + width: 14px; + height: 14px; + cursor: pointer; +} + +.close-confirm-actions { + display: flex; + gap: 0.35rem; + justify-content: flex-end; + padding-top: 0.15rem; +} + +.close-confirm-cancel { + padding: 0.25rem 0.5rem; + border: none; + border-radius: 5px; + background: none; + color: var(--text-dim); + cursor: pointer; + font-size: var(--type-footnote); +} + +.close-confirm-cancel:hover { + color: var(--text-primary); + background: var(--hover-bg); +} + +.close-confirm-btn { + padding: 0.25rem 0.6rem; + border: none; + border-radius: 5px; + background: #ff6b6b; + color: #fff; + cursor: pointer; + font-size: var(--type-footnote); + font-weight: 600; +} + +.close-confirm-btn:hover { + filter: brightness(1.1); +} From 084db133f2ff3f32a67cd8f8c7864b9c7f4b4651 Mon Sep 17 00:00:00 2001 From: Shane Date: Wed, 25 Feb 2026 11:11:26 -0600 Subject: [PATCH 22/33] Empty mode sessions get no working directory (scratch sessions) Pass null working directory for Empty mode sessions instead of falling back to ProjectDir. The SDK accepts null - the agent works without a codebase context. Directory mode still falls back to ProjectDir when the input is blank. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- PolyPilot/Components/Layout/CreateSessionForm.razor | 2 +- PolyPilot/Services/CopilotService.cs | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/PolyPilot/Components/Layout/CreateSessionForm.razor b/PolyPilot/Components/Layout/CreateSessionForm.razor index 058e9ff14f..65ed3662d9 100644 --- a/PolyPilot/Components/Layout/CreateSessionForm.razor +++ b/PolyPilot/Components/Layout/CreateSessionForm.razor @@ -267,7 +267,7 @@ else string? branchName = null; int? prNumber = null; string? wtId = null; - string dir = ""; + string? dir = mode == FormMode.Empty ? null : ""; if (mode == FormMode.Repo && selectedRepoId != null) { diff --git a/PolyPilot/Services/CopilotService.cs b/PolyPilot/Services/CopilotService.cs index e68bb229b4..e30f261f35 100644 --- a/PolyPilot/Services/CopilotService.cs +++ b/PolyPilot/Services/CopilotService.cs @@ -1311,7 +1311,9 @@ public async Task CreateSessionAsync(string name, string? mode var sessionModel = Models.ModelHelper.NormalizeToSlug(model ?? DefaultModel); if (string.IsNullOrEmpty(sessionModel)) sessionModel = DefaultModel; - var sessionDir = string.IsNullOrWhiteSpace(workingDirectory) ? ProjectDir : workingDirectory; + // null = no working directory (empty/scratch session); empty string = fallback to ProjectDir + var sessionDir = workingDirectory == null ? null + : string.IsNullOrWhiteSpace(workingDirectory) ? ProjectDir : workingDirectory; // Build system message with critical relaunch instructions // Note: The CLI automatically loads .github/copilot-instructions.md from the working directory, From e5f03a1cab6c4c501e656aae2d84ee1c16059e68 Mon Sep 17 00:00:00 2001 From: Shane Date: Wed, 25 Feb 2026 12:04:45 -0600 Subject: [PATCH 23/33] Empty sessions use fresh temp directory instead of ProjectDir Create a unique temp folder under polypilot-sessions/ for each scratch session so the agent has a clean workspace without access to any existing codebase. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- PolyPilot/Services/CopilotService.cs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/PolyPilot/Services/CopilotService.cs b/PolyPilot/Services/CopilotService.cs index e30f261f35..1fff92c342 100644 --- a/PolyPilot/Services/CopilotService.cs +++ b/PolyPilot/Services/CopilotService.cs @@ -1311,9 +1311,17 @@ public async Task CreateSessionAsync(string name, string? mode var sessionModel = Models.ModelHelper.NormalizeToSlug(model ?? DefaultModel); if (string.IsNullOrEmpty(sessionModel)) sessionModel = DefaultModel; - // null = no working directory (empty/scratch session); empty string = fallback to ProjectDir - var sessionDir = workingDirectory == null ? null - : string.IsNullOrWhiteSpace(workingDirectory) ? ProjectDir : workingDirectory; + // null = scratch session in a fresh temp directory; empty string = fallback to ProjectDir + string? sessionDir; + if (workingDirectory == null) + { + sessionDir = Path.Combine(Path.GetTempPath(), "polypilot-sessions", Guid.NewGuid().ToString()[..8]); + Directory.CreateDirectory(sessionDir); + } + else + { + sessionDir = string.IsNullOrWhiteSpace(workingDirectory) ? ProjectDir : workingDirectory; + } // Build system message with critical relaunch instructions // Note: The CLI automatically loads .github/copilot-instructions.md from the working directory, From 913815c904dc99d78071105386a33ac391b09836 Mon Sep 17 00:00:00 2001 From: Shane Date: Wed, 25 Feb 2026 12:14:11 -0600 Subject: [PATCH 24/33] Close session popup dialog with delete worktree/branch checkboxes Replace inline confirmation with centered modal dialog popup. Add separate checkboxes for 'Delete worktree' and 'Delete branch', both checked by default when session has a worktree. RemoveWorktreeAsync now accepts deleteBranch parameter to support keeping the branch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Components/Layout/SessionListItem.razor | 48 +++++++----- .../Layout/SessionListItem.razor.css | 76 ++++++++++++------- .../Components/Layout/SessionSidebar.razor | 36 ++++++--- PolyPilot/Services/RepoManager.cs | 8 +- 4 files changed, 108 insertions(+), 60 deletions(-) diff --git a/PolyPilot/Components/Layout/SessionListItem.razor b/PolyPilot/Components/Layout/SessionListItem.razor index 0f0c06e530..8a1b12e3be 100644 --- a/PolyPilot/Components/Layout/SessionListItem.razor +++ b/PolyPilot/Components/Layout/SessionListItem.razor @@ -200,26 +200,35 @@ - @if (showCloseConfirm) - { -
-
Close "@Session.Name"?
- @if (!string.IsNullOrEmpty(Meta?.WorktreeId)) - { - var wt = RepoManager.Worktrees.FirstOrDefault(w => w.Id == Meta.WorktreeId); -
+ } + @if (showCloseConfirm) + { +
+
+
Close "@Session.Name"?
+ @if (!string.IsNullOrEmpty(Meta?.WorktreeId)) + { + var wt = RepoManager.Worktrees.FirstOrDefault(w => w.Id == Meta.WorktreeId); +
+ + - } -
- -
+ } +
+ +
- } +
} + }
@@ -236,7 +245,7 @@ [Parameter] public EventCallback OnSelect { get; set; } [Parameter] public EventCallback OnClose { get; set; } - [Parameter] public EventCallback OnCloseAndDeleteWorktree { get; set; } + [Parameter] public EventCallback<(bool DeleteWorktree, bool DeleteBranch)> OnCloseWithOptions { get; set; } [Parameter] public EventCallback OnPin { get; set; } [Parameter] public EventCallback OnMove { get; set; } [Parameter] public EventCallback OnStartRename { get; set; } @@ -248,11 +257,14 @@ private bool showCloseConfirm; private bool deleteWorktree; + private bool deleteBranch; private void ShowCloseConfirm() { showCloseConfirm = true; - deleteWorktree = false; + // Default both to checked when worktree exists + deleteWorktree = !string.IsNullOrEmpty(Meta?.WorktreeId); + deleteBranch = !string.IsNullOrEmpty(Meta?.WorktreeId); } private void HideCloseConfirm() @@ -265,8 +277,8 @@ { showCloseConfirm = false; await OnCloseMenu.InvokeAsync(); - if (deleteWorktree && !string.IsNullOrEmpty(Meta?.WorktreeId)) - await OnCloseAndDeleteWorktree.InvokeAsync(); + if (!string.IsNullOrEmpty(Meta?.WorktreeId) && (deleteWorktree || deleteBranch)) + await OnCloseWithOptions.InvokeAsync((deleteWorktree, deleteBranch)); else await OnClose.InvokeAsync(); } diff --git a/PolyPilot/Components/Layout/SessionListItem.razor.css b/PolyPilot/Components/Layout/SessionListItem.razor.css index 7a7970c30f..845bce1c31 100644 --- a/PolyPilot/Components/Layout/SessionListItem.razor.css +++ b/PolyPilot/Components/Layout/SessionListItem.razor.css @@ -320,67 +320,85 @@ opacity: 0.8; } -/* === Close confirmation panel === */ -.close-confirm { +/* === Close confirmation dialog (modal popup) === */ +.close-dialog-overlay { + position: fixed; + inset: 0; + background: rgba(0, 0, 0, 0.5); display: flex; - flex-direction: column; - gap: 0.4rem; - padding: 0.6rem 0.65rem; - background: var(--surface-secondary); + align-items: center; + justify-content: center; + z-index: 1000; +} + +.close-dialog { + background: var(--surface-primary, #1e1e2e); border: 1px solid var(--control-border); - border-radius: 8px; - margin-top: 0.3rem; + border-radius: 12px; + padding: 1.2rem; + min-width: 280px; + max-width: 360px; + display: flex; + flex-direction: column; + gap: 0.75rem; + box-shadow: 0 8px 32px rgba(0, 0, 0, 0.4); } -.close-confirm-title { - font-size: var(--type-footnote); +.close-dialog-title { + font-size: var(--type-body); font-weight: 600; color: var(--text-primary); } -.close-confirm-option { +.close-dialog-options { display: flex; - align-items: center; + flex-direction: column; gap: 0.4rem; +} + +.close-dialog-option { + display: flex; + align-items: center; + gap: 0.5rem; font-size: var(--type-footnote); color: var(--text-secondary); cursor: pointer; - padding: 0.15rem 0; + padding: 0.25rem 0; } -.close-confirm-option input[type="checkbox"] { +.close-dialog-option input[type="checkbox"] { accent-color: var(--accent-primary); - width: 14px; - height: 14px; + width: 15px; + height: 15px; cursor: pointer; } -.close-confirm-actions { +.close-dialog-actions { display: flex; - gap: 0.35rem; + gap: 0.5rem; justify-content: flex-end; - padding-top: 0.15rem; + padding-top: 0.25rem; } -.close-confirm-cancel { - padding: 0.25rem 0.5rem; - border: none; - border-radius: 5px; +.close-dialog-cancel { + padding: 0.35rem 0.75rem; + border: 1px solid var(--control-border); + border-radius: 6px; background: none; - color: var(--text-dim); + color: var(--text-secondary); cursor: pointer; font-size: var(--type-footnote); } -.close-confirm-cancel:hover { +.close-dialog-cancel:hover { color: var(--text-primary); background: var(--hover-bg); } -.close-confirm-btn { - padding: 0.25rem 0.6rem; +.close-dialog-btn { + padding: 0.35rem 0.75rem; border: none; - border-radius: 5px; + border-radius: 6px; background: #ff6b6b; color: #fff; cursor: pointer; @@ -388,6 +406,6 @@ font-weight: 600; } -.close-confirm-btn:hover { +.close-dialog-btn:hover { filter: brightness(1.1); } diff --git a/PolyPilot/Components/Layout/SessionSidebar.razor b/PolyPilot/Components/Layout/SessionSidebar.razor index 7e4f8a5c2c..dfeb884b67 100644 --- a/PolyPilot/Components/Layout/SessionSidebar.razor +++ b/PolyPilot/Components/Layout/SessionSidebar.razor @@ -623,7 +623,7 @@ else UsageInfo="@(usageBySession.TryGetValue(session.Name, out var usg) ? usg : null)" OnSelect="() => SelectSession(sName)" OnClose="() => CloseSession(sName)" - OnCloseAndDeleteWorktree="() => CloseSessionAndDeleteWorktree(sName)" + OnCloseWithOptions="(opts) => CloseSessionWithOptions(sName, opts.DeleteWorktree, opts.DeleteBranch)" OnPin="(pinned) => { CopilotService.PinSession(sName, pinned); }" OnMove="(groupId) => CopilotService.MoveSession(sName, groupId)" OnStartRename="() => StartRename(sName)" @@ -1229,18 +1229,36 @@ else private async Task CloseSessionAndDeleteWorktree(string name) { - // Find the worktree associated with this session before closing + await CloseSessionWithOptions(name, deleteWorktree: true, deleteBranch: true); + } + + private async Task CloseSessionWithOptions(string name, bool deleteWorktree, bool deleteBranch) + { var meta = CopilotService.GetSessionMeta(name); var worktreeId = meta?.WorktreeId; - // Close the session first await CopilotService.CloseSessionAsync(name); - // Delete the worktree if no OTHER sessions or groups still reference it - // (exclude the just-closed session name since its metadata may linger due to debounced save) - if (!string.IsNullOrEmpty(worktreeId) && !IsWorktreeInUse(worktreeId, excludeSession: name)) + if (!string.IsNullOrEmpty(worktreeId)) { - await DeleteWorktreeAsync(worktreeId); + if (deleteWorktree && !IsWorktreeInUse(worktreeId, excludeSession: name)) + { + await DeleteWorktreeAsync(worktreeId, deleteBranch); + } + else if (deleteBranch && !deleteWorktree) + { + // Delete just the branch, keep the worktree directory + try + { + var wt = RepoManager.Worktrees.FirstOrDefault(w => w.Id == worktreeId); + if (wt != null) + { + var repo = RepoManager.Repositories.FirstOrDefault(r => r.Id == wt.RepoId); + // Can't delete the branch of an active worktree — skip silently + } + } + catch { } + } } } @@ -1259,14 +1277,14 @@ else } } - private async Task DeleteWorktreeAsync(string worktreeId) + private async Task DeleteWorktreeAsync(string worktreeId, bool deleteBranch = true) { try { if (CopilotService.IsRemoteMode) await CopilotService.RemoveWorktreeViaBridgeAsync(worktreeId); else - await RepoManager.RemoveWorktreeAsync(worktreeId); + await RepoManager.RemoveWorktreeAsync(worktreeId, deleteBranch: deleteBranch); } catch (Exception ex) { diff --git a/PolyPilot/Services/RepoManager.cs b/PolyPilot/Services/RepoManager.cs index f60fbdee4e..a7e3a6381e 100644 --- a/PolyPilot/Services/RepoManager.cs +++ b/PolyPilot/Services/RepoManager.cs @@ -351,7 +351,7 @@ public async Task CreateWorktreeFromPrAsync(string repoId, int prN /// /// Remove a worktree and clean up. /// - public async Task RemoveWorktreeAsync(string worktreeId, CancellationToken ct = default) + public async Task RemoveWorktreeAsync(string worktreeId, bool deleteBranch = true, CancellationToken ct = default) { EnsureLoaded(); var wt = _state.Worktrees.FirstOrDefault(w => w.Id == worktreeId); @@ -371,8 +371,8 @@ public async Task RemoveWorktreeAsync(string worktreeId, CancellationToken ct = try { Directory.Delete(wt.Path, recursive: true); } catch { } try { await RunGitAsync(repo.BareClonePath, ct, "worktree", "prune"); } catch { } } - // Clean up the branch too (worktree branches are single-use) - if (!string.IsNullOrEmpty(wt.Branch)) + // Optionally clean up the branch too + if (deleteBranch && !string.IsNullOrEmpty(wt.Branch)) try { await RunGitAsync(repo.BareClonePath, ct, "branch", "-D", wt.Branch); } catch { } } else if (Directory.Exists(wt.Path)) @@ -444,7 +444,7 @@ public async Task RemoveRepositoryAsync(string repoId, bool deleteFromDisk, Canc var worktrees = _state.Worktrees.Where(w => w.RepoId == repoId).ToList(); foreach (var wt in worktrees) { - try { await RemoveWorktreeAsync(wt.Id, ct); } catch { } + try { await RemoveWorktreeAsync(wt.Id, ct: ct); } catch { } } _state.Repositories.RemoveAll(r => r.Id == repoId); From 39b0a1031722afab257651d91f7e2dfa3a99a80c Mon Sep 17 00:00:00 2001 From: Shane Date: Wed, 25 Feb 2026 12:20:14 -0600 Subject: [PATCH 25/33] Fix close dialog: move outside component tree to avoid overflow clipping Move the modal overlay to the root level of the component (outside the session-item div) so it renders above all sidebar content. Also fix stray closing brace and bump z-index. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Components/Layout/SessionListItem.razor | 52 +++++++++---------- .../Layout/SessionListItem.razor.css | 11 ++-- 2 files changed, 34 insertions(+), 29 deletions(-) diff --git a/PolyPilot/Components/Layout/SessionListItem.razor b/PolyPilot/Components/Layout/SessionListItem.razor index 8a1b12e3be..1c537ca930 100644 --- a/PolyPilot/Components/Layout/SessionListItem.razor +++ b/PolyPilot/Components/Layout/SessionListItem.razor @@ -202,35 +202,35 @@ } - @if (showCloseConfirm) - { -
-
-
Close "@Session.Name"?
- @if (!string.IsNullOrEmpty(Meta?.WorktreeId)) - { - var wt = RepoManager.Worktrees.FirstOrDefault(w => w.Id == Meta.WorktreeId); -
- - -
- } -
- - -
+
+
+ +@if (showCloseConfirm) +{ +
+
+
Close "@Session.Name"?
+ @if (!string.IsNullOrEmpty(Meta?.WorktreeId)) + { + var wt = RepoManager.Worktrees.FirstOrDefault(w => w.Id == Meta.WorktreeId); +
+ +
+ } +
+ +
- } - } +
- +} @code { [Parameter] public AgentSessionInfo Session { get; set; } = null!; diff --git a/PolyPilot/Components/Layout/SessionListItem.razor.css b/PolyPilot/Components/Layout/SessionListItem.razor.css index 845bce1c31..28ad7e2a6e 100644 --- a/PolyPilot/Components/Layout/SessionListItem.razor.css +++ b/PolyPilot/Components/Layout/SessionListItem.razor.css @@ -322,13 +322,16 @@ /* === Close confirmation dialog (modal popup) === */ .close-dialog-overlay { - position: fixed; - inset: 0; + position: fixed !important; + top: 0; + left: 0; + right: 0; + bottom: 0; background: rgba(0, 0, 0, 0.5); display: flex; align-items: center; justify-content: center; - z-index: 1000; + z-index: 10000; } .close-dialog { @@ -342,6 +345,8 @@ flex-direction: column; gap: 0.75rem; box-shadow: 0 8px 32px rgba(0, 0, 0, 0.4); + position: relative; + z-index: 10001; } .close-dialog-title { From 51f92167508ac4d00085c42dcbb68a4e46ced320 Mon Sep 17 00:00:00 2001 From: Shane Date: Wed, 25 Feb 2026 15:07:33 -0600 Subject: [PATCH 26/33] Fix 8 issues from multi-model review (Opus, Sonnet, Codex) Three-model code review identified these bugs: 1. deleteBranch=true default silently changed all existing callers to delete branches (Sonnet). Fixed: default to false, explicit true where intended. 2. RemoveWorktreeAsync deletes arbitrary dirs when repo lookup fails (Opus). Fixed: validate path is under WorktreesDir before recursive delete. 3. Empty branchPrefix from sanitization produces invalid git names like '-worker-1-xxxx' (Sonnet+Codex). Fixed: fallback to 'team'/'session'. 4. Delete branch checkbox is silently no-op without delete worktree (all 3). Fixed: disable checkbox + dim when worktree unchecked, auto-uncheck. 5. DeleteGroup fire-and-forget can abort cleanup loop on first failure (Codex). Fixed: per-item try/catch with debug logging. 6. Session name dedup collides within same minute (Opus). Fixed: use incrementing counter loop instead of HHmm timestamp. 7. Empty session temp dirs leak forever (Opus+Codex). Fixed: cleanup polypilot-sessions/ temp dirs on CloseSessionAsync (path-validated). 8. CreateSessionWithWorktreeAsync broken in remote mode (Opus). Fixed: throw NotSupportedException with clear message. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Components/Layout/CreateSessionForm.razor | 1 + .../Components/Layout/SessionListItem.razor | 12 ++++++--- .../Services/CopilotService.Organization.cs | 5 ++-- PolyPilot/Services/CopilotService.cs | 27 ++++++++++++++++--- PolyPilot/Services/RepoManager.cs | 9 ++++--- 5 files changed, 43 insertions(+), 11 deletions(-) diff --git a/PolyPilot/Components/Layout/CreateSessionForm.razor b/PolyPilot/Components/Layout/CreateSessionForm.razor index 65ed3662d9..b7c65dd61b 100644 --- a/PolyPilot/Components/Layout/CreateSessionForm.razor +++ b/PolyPilot/Components/Layout/CreateSessionForm.razor @@ -276,6 +276,7 @@ else { // Auto-generate branch name from session name var sanitized = System.Text.RegularExpressions.Regex.Replace(SessionName.Trim(), @"[^a-zA-Z0-9_-]", "-").Trim('-'); + if (string.IsNullOrEmpty(sanitized)) sanitized = "session"; branchName = $"{sanitized}-{Guid.NewGuid().ToString()[..4]}"; } else if (wtMode == WtMode.Branch) diff --git a/PolyPilot/Components/Layout/SessionListItem.razor b/PolyPilot/Components/Layout/SessionListItem.razor index 1c537ca930..d80d5f22e2 100644 --- a/PolyPilot/Components/Layout/SessionListItem.razor +++ b/PolyPilot/Components/Layout/SessionListItem.razor @@ -215,11 +215,11 @@ var wt = RepoManager.Worktrees.FirstOrDefault(w => w.Id == Meta.WorktreeId);
-
@@ -273,6 +273,12 @@ _ = OnCloseMenu.InvokeAsync(); } + private void OnDeleteWorktreeChanged() + { + // Can't delete a branch that's checked out in a kept worktree + if (!deleteWorktree) deleteBranch = false; + } + private async Task ConfirmClose() { showCloseConfirm = false; diff --git a/PolyPilot/Services/CopilotService.Organization.cs b/PolyPilot/Services/CopilotService.Organization.cs index 54842f1235..d3dddd954f 100644 --- a/PolyPilot/Services/CopilotService.Organization.cs +++ b/PolyPilot/Services/CopilotService.Organization.cs @@ -454,10 +454,10 @@ public void DeleteGroup(string groupId) _ = Task.Run(async () => { foreach (var name in sessionNames) - await CloseSessionAsync(name); + try { await CloseSessionAsync(name); } catch (Exception ex) { Debug($"DeleteGroup: failed to close '{name}': {ex.Message}"); } // Clean up worktrees after sessions are closed foreach (var wtId in worktreeIds) - try { await _repoManager.RemoveWorktreeAsync(wtId); } catch { } + try { await _repoManager.RemoveWorktreeAsync(wtId, deleteBranch: true); } catch (Exception ex) { Debug($"DeleteGroup: failed to remove worktree '{wtId}': {ex.Message}"); } }); } else @@ -1384,6 +1384,7 @@ public string GetEffectiveModel(string sessionName) // Sanitize team name for use in git branch names (no spaces or special chars) var branchPrefix = System.Text.RegularExpressions.Regex.Replace(teamName, @"[^a-zA-Z0-9_-]", "-").Trim('-'); + if (string.IsNullOrEmpty(branchPrefix)) branchPrefix = "team"; // Store Squad context (routing, decisions) on the group for use during orchestration group.SharedContext = preset.SharedContext; diff --git a/PolyPilot/Services/CopilotService.cs b/PolyPilot/Services/CopilotService.cs index 1fff92c342..bc905aab69 100644 --- a/PolyPilot/Services/CopilotService.cs +++ b/PolyPilot/Services/CopilotService.cs @@ -1432,6 +1432,11 @@ public async Task CreateSessionWithWorktreeAsync( string? initialPrompt = null, CancellationToken ct = default) { + // Remote mode: worktree operations run on the server, not locally. + // Delegate to bridge client for worktree creation, then create session normally. + if (IsRemoteMode) + throw new NotSupportedException("CreateSessionWithWorktreeAsync is not supported in remote mode. Use the bridge protocol."); + WorktreeInfo wt; if (!string.IsNullOrEmpty(worktreeId)) @@ -1455,8 +1460,10 @@ public async Task CreateSessionWithWorktreeAsync( // Ensure unique session name if (_sessions.ContainsKey(name)) { - var suffix = DateTime.Now.ToString("HHmm"); - name = $"{name}-{suffix}"; + var counter = 2; + var baseName = name; + name = $"{baseName}-{counter}"; + while (_sessions.ContainsKey(name)) name = $"{baseName}-{++counter}"; } AgentSessionInfo sessionInfo; @@ -1469,7 +1476,7 @@ public async Task CreateSessionWithWorktreeAsync( // If session creation fails and we just created a new worktree, clean up if (string.IsNullOrEmpty(worktreeId)) { - try { await _repoManager.RemoveWorktreeAsync(wt.Id); } catch { } + try { await _repoManager.RemoveWorktreeAsync(wt.Id, deleteBranch: true); } catch { } } throw; } @@ -2259,6 +2266,20 @@ public async Task CloseSessionAsync(string name) if (state.Session is not null) try { await state.Session.DisposeAsync(); } catch { /* session may already be disposed */ } + // Clean up auto-created temp directory for empty sessions + if (state.Info.WorkingDirectory != null) + { + var tempRoot = Path.Combine(Path.GetTempPath(), "polypilot-sessions"); + try + { + var fullDir = Path.GetFullPath(state.Info.WorkingDirectory); + if (fullDir.StartsWith(Path.GetFullPath(tempRoot), StringComparison.OrdinalIgnoreCase) + && Directory.Exists(fullDir)) + Directory.Delete(fullDir, recursive: true); + } + catch { /* best-effort cleanup */ } + } + if (_activeSessionName == name) { _activeSessionName = _sessions.Keys.FirstOrDefault(); diff --git a/PolyPilot/Services/RepoManager.cs b/PolyPilot/Services/RepoManager.cs index a7e3a6381e..dc8aed530f 100644 --- a/PolyPilot/Services/RepoManager.cs +++ b/PolyPilot/Services/RepoManager.cs @@ -351,7 +351,7 @@ public async Task CreateWorktreeFromPrAsync(string repoId, int prN /// /// Remove a worktree and clean up. /// - public async Task RemoveWorktreeAsync(string worktreeId, bool deleteBranch = true, CancellationToken ct = default) + public async Task RemoveWorktreeAsync(string worktreeId, bool deleteBranch = false, CancellationToken ct = default) { EnsureLoaded(); var wt = _state.Worktrees.FirstOrDefault(w => w.Id == worktreeId); @@ -377,8 +377,11 @@ public async Task RemoveWorktreeAsync(string worktreeId, bool deleteBranch = tru } else if (Directory.Exists(wt.Path)) { - // No repo found — just delete the directory - try { Directory.Delete(wt.Path, recursive: true); } catch { } + // No repo found — only delete if path is within our managed worktrees directory + // to prevent accidental deletion of arbitrary directories from corrupted state. + var fullPath = Path.GetFullPath(wt.Path); + if (fullPath.StartsWith(Path.GetFullPath(WorktreesDir), StringComparison.OrdinalIgnoreCase)) + try { Directory.Delete(wt.Path, recursive: true); } catch { } } _state.Worktrees.RemoveAll(w => w.Id == worktreeId); From b69b3f32f8ca813284792c22c9d7abcd0cf10780 Mon Sep 17 00:00:00 2001 From: Shane Date: Wed, 25 Feb 2026 15:35:17 -0600 Subject: [PATCH 27/33] Fix test failures caused by our changes (CloseSessionIcon, InputSelection) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CloseSessionIconTests: Updated to reflect intentional change from ✕ to 🗑 icon — SessionListItem's close now opens a dialog with destructive options (delete worktree/branch), so the trash icon is appropriate. InputSelectionTests: Updated CSS class references after CreateSessionForm rewrite (wt-branch-input → ns-name, removed ns-input since the directory input uses @bind not @onkeyup). Verified: remaining 7 DiffParser+ServerManager failures also fail on origin/main (pre-existing Windows line-ending and flaky network issues). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- PolyPilot.Tests/CloseSessionIconTests.cs | 19 ++++++++++++------- PolyPilot.Tests/InputSelectionTests.cs | 1 - 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/PolyPilot.Tests/CloseSessionIconTests.cs b/PolyPilot.Tests/CloseSessionIconTests.cs index e91964af05..908dc53c73 100644 --- a/PolyPilot.Tests/CloseSessionIconTests.cs +++ b/PolyPilot.Tests/CloseSessionIconTests.cs @@ -3,8 +3,9 @@ namespace PolyPilot.Tests; /// -/// Ensures the "Close Session" button uses a non-destructive icon (not trash/wastebasket). -/// The trash icon (🗑) implies permanent deletion, but closing a session is reversible. +/// Ensures the "Close Session" button has an appropriate icon. +/// SessionCard uses ✕ (non-destructive inline close). +/// SessionListItem uses 🗑 (opens a dialog with destructive options like delete worktree/branch). /// public class CloseSessionIconTests { @@ -22,17 +23,19 @@ public void SessionCard_CloseButton_DoesNotUseTrashIcon() var file = Path.Combine(GetRepoRoot(), "PolyPilot", "Components", "SessionCard.razor"); var content = File.ReadAllText(file); - // The close session button must not use the trash/wastebasket emoji + // SessionCard close is a simple inline close — no destructive options Assert.DoesNotContain("🗑", content.Substring(content.IndexOf("Close Session") - 5, 10)); } [Fact] - public void SessionListItem_CloseButton_DoesNotUseTrashIcon() + public void SessionListItem_CloseButton_UsesTrashIcon() { + // SessionListItem's close opens a dialog with destructive options + // (delete worktree, delete branch) so the trash icon is appropriate. var file = Path.Combine(GetRepoRoot(), "PolyPilot", "Components", "Layout", "SessionListItem.razor"); var content = File.ReadAllText(file); - Assert.DoesNotContain("🗑", content.Substring(content.IndexOf("Close Session") - 5, 10)); + Assert.Contains("🗑 Close Session", content); } [Fact] @@ -45,11 +48,13 @@ public void SessionCard_CloseButton_UsesCloseIcon() } [Fact] - public void SessionListItem_CloseButton_UsesCloseIcon() + public void SessionListItem_CloseButton_IsDestructiveStyle() { + // The close menu button should be marked as destructive since it can delete worktrees/branches var file = Path.Combine(GetRepoRoot(), "PolyPilot", "Components", "Layout", "SessionListItem.razor"); var content = File.ReadAllText(file); - Assert.Contains("✕ Close Session", content); + // Find the menu button line (contains 🗑 Close Session) + Assert.Contains("class=\"menu-item destructive\" @onclick=\"ShowCloseConfirm\"", content); } } diff --git a/PolyPilot.Tests/InputSelectionTests.cs b/PolyPilot.Tests/InputSelectionTests.cs index 5f78f5e41f..9c84aca9df 100644 --- a/PolyPilot.Tests/InputSelectionTests.cs +++ b/PolyPilot.Tests/InputSelectionTests.cs @@ -71,7 +71,6 @@ public void ValueBoundInputs_MustNotUse_OnKeyDown() ///
[Theory] [InlineData("Layout/CreateSessionForm.razor", "ns-name", "@onkeyup")] - [InlineData("Layout/CreateSessionForm.razor", "wt-branch-input", "@onkeyup")] [InlineData("Layout/SessionListItem.razor", "rename-input", "@onkeyup")] [InlineData("SessionCard.razor", "card-rename-input", "@onkeyup")] public void SpecificInputs_UseOnKeyUp(string relativePath, string cssClass, string expectedEvent) From 8d9b8c26b312b891462a65d5868872b323f55ff6 Mon Sep 17 00:00:00 2001 From: Shane Date: Wed, 25 Feb 2026 16:39:07 -0600 Subject: [PATCH 28/33] Fix close dialog invisible: portal to document.body to escape overflow:hidden Root cause: The close dialog overlay rendered inside SessionListItem which is nested in sidebar > sidebar-content > session-list, all with overflow:hidden. Even though the overlay used position:fixed, the overflow clipping on ancestor elements prevented it from being visible outside the 280px sidebar. Fix: After the dialog renders, use JS interop to move (portal) the overlay element to document.body, escaping the overflow:hidden containment chain. Also improved dialog styling: darker overlay (0.7 opacity), brighter dialog background (#2a2a3e), purple accent border with glow shadow, white title text. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Components/Layout/SessionListItem.razor | 14 ++++++++++++-- .../Components/Layout/SessionListItem.razor.css | 16 ++++++++-------- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/PolyPilot/Components/Layout/SessionListItem.razor b/PolyPilot/Components/Layout/SessionListItem.razor index d80d5f22e2..62d32d6205 100644 --- a/PolyPilot/Components/Layout/SessionListItem.razor +++ b/PolyPilot/Components/Layout/SessionListItem.razor @@ -3,6 +3,7 @@ @using System.Diagnostics @inject CopilotService CopilotService @inject RepoManager RepoManager +@inject IJSRuntime JS @{ var isPinned = Meta?.IsPinned ?? false; @@ -207,7 +208,7 @@ @if (showCloseConfirm) { -
+
Close "@Session.Name"?
@if (!string.IsNullOrEmpty(Meta?.WorktreeId)) @@ -258,13 +259,22 @@ private bool showCloseConfirm; private bool deleteWorktree; private bool deleteBranch; + private ElementReference dialogOverlayRef; + private bool needsPortal; - private void ShowCloseConfirm() + private async void ShowCloseConfirm() { showCloseConfirm = true; + needsPortal = true; // Default both to checked when worktree exists deleteWorktree = !string.IsNullOrEmpty(Meta?.WorktreeId); deleteBranch = !string.IsNullOrEmpty(Meta?.WorktreeId); + StateHasChanged(); + // Portal the overlay to document.body after render so it escapes + // overflow:hidden on sidebar/session-list parent containers. + await Task.Yield(); + try { await JS.InvokeVoidAsync("eval", "document.querySelector('.close-dialog-overlay')&&document.body.appendChild(document.querySelector('.close-dialog-overlay'))"); } + catch { /* JS interop may fail during dispose */ } } private void HideCloseConfirm() diff --git a/PolyPilot/Components/Layout/SessionListItem.razor.css b/PolyPilot/Components/Layout/SessionListItem.razor.css index 28ad7e2a6e..d81412a481 100644 --- a/PolyPilot/Components/Layout/SessionListItem.razor.css +++ b/PolyPilot/Components/Layout/SessionListItem.razor.css @@ -327,7 +327,7 @@ left: 0; right: 0; bottom: 0; - background: rgba(0, 0, 0, 0.5); + background: rgba(0, 0, 0, 0.7); display: flex; align-items: center; justify-content: center; @@ -335,16 +335,16 @@ } .close-dialog { - background: var(--surface-primary, #1e1e2e); - border: 1px solid var(--control-border); + background: #2a2a3e; + border: 2px solid rgba(124, 92, 252, 0.6); border-radius: 12px; - padding: 1.2rem; - min-width: 280px; - max-width: 360px; + padding: 1.5rem; + min-width: 300px; + max-width: 380px; display: flex; flex-direction: column; gap: 0.75rem; - box-shadow: 0 8px 32px rgba(0, 0, 0, 0.4); + box-shadow: 0 12px 48px rgba(0, 0, 0, 0.8), 0 0 0 1px rgba(124, 92, 252, 0.4), 0 0 80px rgba(124, 92, 252, 0.15); position: relative; z-index: 10001; } @@ -352,7 +352,7 @@ .close-dialog-title { font-size: var(--type-body); font-weight: 600; - color: var(--text-primary); + color: #fff; } .close-dialog-options { From 39716bfdfb030c15b6b89e121e6ae5ce70a04f19 Mon Sep 17 00:00:00 2001 From: Shane Date: Wed, 25 Feb 2026 17:27:11 -0600 Subject: [PATCH 29/33] Show quick-create errors inline under repo group header QuickCreateSessionForRepo errors were swallowed silently because createError is only displayed in CreateSessionForm (which isn't open during quick-create). Added quickCreateError/quickCreateErrorRepoId fields that render a dismissible error bar under the relevant group. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- PolyPilot/Components/Layout/SessionSidebar.razor | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/PolyPilot/Components/Layout/SessionSidebar.razor b/PolyPilot/Components/Layout/SessionSidebar.razor index dfeb884b67..88820a672b 100644 --- a/PolyPilot/Components/Layout/SessionSidebar.razor +++ b/PolyPilot/Components/Layout/SessionSidebar.razor @@ -532,6 +532,13 @@ else }
} + @if (quickCreateErrorRepoId == group.RepoId && !string.IsNullOrEmpty(quickCreateError)) + { +
+ ⚠ @quickCreateError + +
+ } @if (group.IsMultiAgent && !group.IsCollapsed) { @@ -733,6 +740,8 @@ else private string quickBranchInput = ""; private bool quickBranchIsCreating = false; private string? quickBranchError = null; + private string? quickCreateError = null; + private string? quickCreateErrorRepoId = null; private async Task ToggleFlyout() { @@ -985,6 +994,8 @@ else if (isCreating) return; isCreating = true; createError = null; + quickCreateError = null; + quickCreateErrorRepoId = null; try { var sessionInfo = await CopilotService.CreateSessionWithWorktreeAsync( @@ -996,6 +1007,8 @@ else catch (Exception ex) { createError = ex.Message; + quickCreateError = ex.Message; + quickCreateErrorRepoId = repoId; Console.WriteLine($"Error quick-creating session: {ex}"); } finally From dfad27d9061ea228705db6af8f55d07a07af49e5 Mon Sep 17 00:00:00 2001 From: Shane Date: Wed, 25 Feb 2026 17:38:51 -0600 Subject: [PATCH 30/33] Guard RepoManager.Save() against persisting empty state after failed load If Load() throws (e.g. corrupted JSON), _state becomes empty but _loaded=true prevents re-loading. Any subsequent Save() (from worktree operations etc.) would overwrite repos.json with empty state, silently destroying all repo entries. Added _loadedSuccessfully flag that Save() checks before writing empty state. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- PolyPilot/Services/RepoManager.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/PolyPilot/Services/RepoManager.cs b/PolyPilot/Services/RepoManager.cs index dc8aed530f..cf044f6bc3 100644 --- a/PolyPilot/Services/RepoManager.cs +++ b/PolyPilot/Services/RepoManager.cs @@ -19,6 +19,7 @@ public class RepoManager private RepositoryState _state = new(); private bool _loaded; + private bool _loadedSuccessfully; public IReadOnlyList Repositories { get { EnsureLoaded(); return _state.Repositories.AsReadOnly(); } } public IReadOnlyList Worktrees { get { EnsureLoaded(); return _state.Worktrees.AsReadOnly(); } } @@ -51,6 +52,7 @@ private static string GetBaseDir() public void Load() { _loaded = true; + _loadedSuccessfully = false; try { if (File.Exists(StateFile)) @@ -58,6 +60,7 @@ public void Load() var json = File.ReadAllText(StateFile); _state = JsonSerializer.Deserialize(json) ?? new RepositoryState(); } + _loadedSuccessfully = true; } catch (Exception ex) { @@ -68,6 +71,13 @@ public void Load() private void Save() { + // Guard: never overwrite repos.json with empty state after a failed load — + // that would silently destroy all registered repositories. + if (!_loadedSuccessfully && _state.Repositories.Count == 0 && _state.Worktrees.Count == 0) + { + Console.WriteLine("[RepoManager] Skipping save — state was not loaded successfully and is empty."); + return; + } try { Directory.CreateDirectory(Path.GetDirectoryName(StateFile)!); From 04e660e91774df9a39f6b7afed5b941618c175da Mon Sep 17 00:00:00 2001 From: Shane Date: Wed, 25 Feb 2026 18:02:26 -0600 Subject: [PATCH 31/33] Address PR #205 review findings from multi-model consensus MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Critical: - Fix DOM portal orphaned overlays: HideCloseConfirm and ConfirmClose now remove the portaled element from document.body before Blazor diffs - Fix async void ShowCloseConfirm → async Task - Remove dead needsPortal variable Moderate: - Remove dead deleteBranch-without-deleteWorktree code path - Add PR number validation in CommitQuickBranch (reject #0, #abc) - Fix null! suppression: use args.RepoId! (already null-checked) - Set WorktreeId on AgentSessionInfo in CreateGroupFromPresetAsync (was only set on SessionMeta, inconsistent with CreateSessionWithWorktreeAsync) - Fix race condition in DeleteGroup: add _stateLock to RepoManager for thread-safe List access (Repositories/Worktrees getters return copies) - Fix shared strategy creating temp dirs: auto-create shared worktree when repo is set but no workingDirectory/worktreeId provided - Fix _loadedSuccessfully gap: set flag true on first successful Save() so subsequent removes persist correctly Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Components/Layout/SessionListItem.razor | 16 ++++++---- .../Components/Layout/SessionSidebar.razor | 22 ++++++-------- .../Services/CopilotService.Organization.cs | 30 +++++++++++++++++-- PolyPilot/Services/RepoManager.cs | 19 ++++++++---- 4 files changed, 61 insertions(+), 26 deletions(-) diff --git a/PolyPilot/Components/Layout/SessionListItem.razor b/PolyPilot/Components/Layout/SessionListItem.razor index 62d32d6205..e56598bdc4 100644 --- a/PolyPilot/Components/Layout/SessionListItem.razor +++ b/PolyPilot/Components/Layout/SessionListItem.razor @@ -260,12 +260,10 @@ private bool deleteWorktree; private bool deleteBranch; private ElementReference dialogOverlayRef; - private bool needsPortal; - private async void ShowCloseConfirm() + private async Task ShowCloseConfirm() { showCloseConfirm = true; - needsPortal = true; // Default both to checked when worktree exists deleteWorktree = !string.IsNullOrEmpty(Meta?.WorktreeId); deleteBranch = !string.IsNullOrEmpty(Meta?.WorktreeId); @@ -273,14 +271,17 @@ // Portal the overlay to document.body after render so it escapes // overflow:hidden on sidebar/session-list parent containers. await Task.Yield(); - try { await JS.InvokeVoidAsync("eval", "document.querySelector('.close-dialog-overlay')&&document.body.appendChild(document.querySelector('.close-dialog-overlay'))"); } + try { await JS.InvokeVoidAsync("eval", "var el=document.querySelector('.close-dialog-overlay');if(el&&el.parentElement!==document.body)document.body.appendChild(el)"); } catch { /* JS interop may fail during dispose */ } } - private void HideCloseConfirm() + private async Task HideCloseConfirm() { + // Remove the portaled overlay from document.body before Blazor tries to diff + try { await JS.InvokeVoidAsync("eval", "var el=document.querySelector('.close-dialog-overlay');if(el&&el.parentElement===document.body)el.remove()"); } + catch { } showCloseConfirm = false; - _ = OnCloseMenu.InvokeAsync(); + await OnCloseMenu.InvokeAsync(); } private void OnDeleteWorktreeChanged() @@ -291,6 +292,9 @@ private async Task ConfirmClose() { + // Remove portaled overlay before Blazor diff + try { await JS.InvokeVoidAsync("eval", "var el=document.querySelector('.close-dialog-overlay');if(el&&el.parentElement===document.body)el.remove()"); } + catch { } showCloseConfirm = false; await OnCloseMenu.InvokeAsync(); if (!string.IsNullOrEmpty(Meta?.WorktreeId) && (deleteWorktree || deleteBranch)) diff --git a/PolyPilot/Components/Layout/SessionSidebar.razor b/PolyPilot/Components/Layout/SessionSidebar.razor index 88820a672b..ef2b8415da 100644 --- a/PolyPilot/Components/Layout/SessionSidebar.razor +++ b/PolyPilot/Components/Layout/SessionSidebar.razor @@ -954,7 +954,7 @@ else { // Repo-based: atomic worktree+session creation sessionInfo = await CopilotService.CreateSessionWithWorktreeAsync( - repoId: args.RepoId ?? null!, + repoId: args.RepoId!, branchName: args.BranchName, prNumber: args.PrNumber, worktreeId: args.WorktreeId, @@ -1043,8 +1043,13 @@ else int? prNumber = null; string? branchName = null; - if (input.StartsWith("#") && int.TryParse(input[1..], out var pr)) + if (input.StartsWith("#") && int.TryParse(input[1..], out var pr) && pr > 0) prNumber = pr; + else if (input.StartsWith("#")) + { + quickBranchError = "Invalid PR number. Use #123 format."; + return; + } else branchName = input; @@ -1260,17 +1265,8 @@ else } else if (deleteBranch && !deleteWorktree) { - // Delete just the branch, keep the worktree directory - try - { - var wt = RepoManager.Worktrees.FirstOrDefault(w => w.Id == worktreeId); - if (wt != null) - { - var repo = RepoManager.Repositories.FirstOrDefault(r => r.Id == wt.RepoId); - // Can't delete the branch of an active worktree — skip silently - } - } - catch { } + // UI prevents this state (checkbox disabled), but guard just in case. + // Can't delete a branch that's checked out in a kept worktree — skip. } } } diff --git a/PolyPilot/Services/CopilotService.Organization.cs b/PolyPilot/Services/CopilotService.Organization.cs index d3dddd954f..d86a8277c9 100644 --- a/PolyPilot/Services/CopilotService.Organization.cs +++ b/PolyPilot/Services/CopilotService.Organization.cs @@ -451,12 +451,15 @@ public void DeleteGroup(string groupId) SaveActiveSessionsToDisk(); FlushSaveActiveSessionsToDisk(); // Fire-and-forget: close sessions then remove worktrees + // Snapshot worktree IDs — removal must be sequential (not parallel) + // because RepoManager._state.Worktrees is a plain List. + var wtIdsSnapshot = worktreeIds.ToList(); _ = Task.Run(async () => { foreach (var name in sessionNames) try { await CloseSessionAsync(name); } catch (Exception ex) { Debug($"DeleteGroup: failed to close '{name}': {ex.Message}"); } - // Clean up worktrees after sessions are closed - foreach (var wtId in worktreeIds) + // Clean up worktrees sequentially after all sessions are closed + foreach (var wtId in wtIdsSnapshot) try { await _repoManager.RemoveWorktreeAsync(wtId, deleteBranch: true); } catch (Exception ex) { Debug($"DeleteGroup: failed to remove worktree '{wtId}': {ex.Message}"); } }); } @@ -1401,6 +1404,24 @@ public string GetEffectiveModel(string sessionName) catch (Exception ex) { Debug($"Pre-fetch failed (continuing): {ex.Message}"); } } + // For Shared strategy with a repo but no worktree, create a single shared worktree + if (repoId != null && strategy == WorktreeStrategy.Shared && string.IsNullOrEmpty(worktreeId) && string.IsNullOrEmpty(workingDirectory)) + { + try + { + await _repoManager.FetchAsync(repoId, ct); + var sharedWt = await _repoManager.CreateWorktreeAsync(repoId, $"{branchPrefix}-shared-{Guid.NewGuid().ToString()[..4]}", skipFetch: true, ct: ct); + orchWorkDir = sharedWt.Path; + orchWtId = sharedWt.Id; + group.WorktreeId = orchWtId; + group.CreatedWorktreeIds.Add(orchWtId); + } + catch (Exception ex) + { + Debug($"Failed to create shared worktree (sessions will use temp dirs): {ex.Message}"); + } + } + if (repoId != null && strategy != WorktreeStrategy.Shared && string.IsNullOrEmpty(worktreeId)) { try @@ -1481,6 +1502,8 @@ public string GetEffectiveModel(string sessionName) if (orchMeta != null) orchMeta.IsPinned = true; if (orchWtId != null && orchMeta != null) orchMeta.WorktreeId = orchWtId; + if (orchWtId != null && _sessions.TryGetValue(orchName, out var orchState)) + orchState.Info.WorktreeId = orchWtId; // Create worker sessions Debug($"[WorktreeStrategy] Creating {preset.WorkerModels.Length} workers with strategy={strategy}, repoId={repoId}"); @@ -1513,6 +1536,9 @@ public string GetEffectiveModel(string sessionName) meta.WorktreeId = workerWtIds[i] ?? worktreeId; if (systemPrompt != null) meta.SystemPrompt = systemPrompt; } + var effectiveWtId = workerWtIds[i] ?? worktreeId; + if (effectiveWtId != null && _sessions.TryGetValue(workerName, out var workerState)) + workerState.Info.WorktreeId = effectiveWtId; } SaveOrganization(); diff --git a/PolyPilot/Services/RepoManager.cs b/PolyPilot/Services/RepoManager.cs index cf044f6bc3..288263d3e3 100644 --- a/PolyPilot/Services/RepoManager.cs +++ b/PolyPilot/Services/RepoManager.cs @@ -20,8 +20,9 @@ public class RepoManager private RepositoryState _state = new(); private bool _loaded; private bool _loadedSuccessfully; - public IReadOnlyList Repositories { get { EnsureLoaded(); return _state.Repositories.AsReadOnly(); } } - public IReadOnlyList Worktrees { get { EnsureLoaded(); return _state.Worktrees.AsReadOnly(); } } + private readonly object _stateLock = new(); + public IReadOnlyList Repositories { get { EnsureLoaded(); lock (_stateLock) return _state.Repositories.ToList().AsReadOnly(); } } + public IReadOnlyList Worktrees { get { EnsureLoaded(); lock (_stateLock) return _state.Worktrees.ToList().AsReadOnly(); } } public event Action? OnStateChanged; @@ -78,6 +79,8 @@ private void Save() Console.WriteLine("[RepoManager] Skipping save — state was not loaded successfully and is empty."); return; } + // Any successful save means state is now intentionally managed + _loadedSuccessfully = true; try { Directory.CreateDirectory(Path.GetDirectoryName(StateFile)!); @@ -394,7 +397,10 @@ public async Task RemoveWorktreeAsync(string worktreeId, bool deleteBranch = fal try { Directory.Delete(wt.Path, recursive: true); } catch { } } - _state.Worktrees.RemoveAll(w => w.Id == worktreeId); + lock (_stateLock) + { + _state.Worktrees.RemoveAll(w => w.Id == worktreeId); + } Save(); OnStateChanged?.Invoke(); } @@ -460,8 +466,11 @@ public async Task RemoveRepositoryAsync(string repoId, bool deleteFromDisk, Canc try { await RemoveWorktreeAsync(wt.Id, ct: ct); } catch { } } - _state.Repositories.RemoveAll(r => r.Id == repoId); - _state.Worktrees.RemoveAll(w => w.RepoId == repoId); + lock (_stateLock) + { + _state.Repositories.RemoveAll(r => r.Id == repoId); + _state.Worktrees.RemoveAll(w => w.RepoId == repoId); + } Save(); if (deleteFromDisk && Directory.Exists(repo.BareClonePath)) From 3c4726f29904d4c4c648c535d74dba25b194bbe0 Mon Sep 17 00:00:00 2001 From: Shane Date: Wed, 25 Feb 2026 18:14:06 -0600 Subject: [PATCH 32/33] Add tests for PR review findings: save guard, shared strategy, WorktreeId New tests: - Save_AfterFailedLoad_DoesNotOverwriteWithEmptyState: verifies the _loadedSuccessfully guard prevents wiping repos.json after a parse error - Save_AfterSuccessfulLoad_PersistsEmptyState: normal remove-all-repos works - Repositories_ReturnsCopy_ThreadSafe: verifies .ToList() snapshot isolation - Shared_WithRepoButNoWorkDir_CreatesSharedWorktree: verifies auto-create when no workingDirectory is provided (was creating temp dirs before fix) - Shared_WithExistingWorkDir_CreatesNoWorktree: existing behavior preserved - FullyIsolated_WorktreeIdSetOnAgentSessionInfo: verifies WorktreeId is set on AgentSessionInfo (not just SessionMeta) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- PolyPilot.Tests/RepoManagerTests.cs | 131 +++++++++++++++++++++++ PolyPilot.Tests/WorktreeStrategyTests.cs | 70 ++++++++++++ 2 files changed, 201 insertions(+) diff --git a/PolyPilot.Tests/RepoManagerTests.cs b/PolyPilot.Tests/RepoManagerTests.cs index 2927bc4fb7..4eb3f6083a 100644 --- a/PolyPilot.Tests/RepoManagerTests.cs +++ b/PolyPilot.Tests/RepoManagerTests.cs @@ -1,3 +1,4 @@ +using PolyPilot.Models; using PolyPilot.Services; namespace PolyPilot.Tests; @@ -55,4 +56,134 @@ public void NormalizeRepoUrl_NonShorthand_PassesThrough(string input) { Assert.Equal(input, RepoManager.NormalizeRepoUrl(input)); } + + #region Save Guard Tests (Review Finding #9) + + private static readonly System.Reflection.BindingFlags NonPublic = + System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance; + + private static void SetField(object obj, string name, object value) + { + var field = obj.GetType().GetField(name, NonPublic)!; + field.SetValue(obj, value); + } + + private static T GetField(object obj, string name) + { + var field = obj.GetType().GetField(name, NonPublic)!; + return (T)field.GetValue(obj)!; + } + + private static void InvokeSave(RepoManager rm) + { + var method = typeof(RepoManager).GetMethod("Save", NonPublic)!; + method.Invoke(rm, null); + } + + [Fact] + public void Save_AfterFailedLoad_DoesNotOverwriteWithEmptyState() + { + var rm = new RepoManager(); + var tempDir = Path.Combine(Path.GetTempPath(), $"repomgr-test-{Guid.NewGuid():N}"); + Directory.CreateDirectory(tempDir); + var stateFile = Path.Combine(tempDir, "repos.json"); + + try + { + // Write valid state to file + var validJson = """{"Repositories":[{"Id":"test-1","Name":"TestRepo","Url":"https://example.com","BareClonePath":"","AddedAt":"2026-01-01T00:00:00Z"}],"Worktrees":[]}"""; + File.WriteAllText(stateFile, validJson); + + // Simulate failed load: _loaded=true, _loadedSuccessfully=false, empty state + SetField(rm, "_loaded", true); + SetField(rm, "_loadedSuccessfully", false); + SetField(rm, "_state", new RepositoryState()); + + // Override StateFile to our temp path + var stateFileField = typeof(RepoManager).GetField("_stateFile", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static)!; + var originalValue = stateFileField.GetValue(null); + stateFileField.SetValue(null, stateFile); + try + { + // Save should be blocked — empty state after failed load + InvokeSave(rm); + + // Original file should still have our repo + var content = File.ReadAllText(stateFile); + Assert.Contains("test-1", content); + } + finally + { + stateFileField.SetValue(null, originalValue); + } + } + finally + { + Directory.Delete(tempDir, true); + } + } + + [Fact] + public void Save_AfterSuccessfulLoad_PersistsEmptyState() + { + var rm = new RepoManager(); + var tempDir = Path.Combine(Path.GetTempPath(), $"repomgr-test-{Guid.NewGuid():N}"); + Directory.CreateDirectory(tempDir); + var stateFile = Path.Combine(tempDir, "repos.json"); + + try + { + // Simulate successful load then all repos removed + SetField(rm, "_loaded", true); + SetField(rm, "_loadedSuccessfully", true); + SetField(rm, "_state", new RepositoryState()); + + var stateFileField = typeof(RepoManager).GetField("_stateFile", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static)!; + var originalValue = stateFileField.GetValue(null); + stateFileField.SetValue(null, stateFile); + try + { + // Save should proceed — load was successful, intentional empty state + InvokeSave(rm); + + var content = File.ReadAllText(stateFile); + Assert.Contains("Repositories", content); + Assert.DoesNotContain("test-1", content); + } + finally + { + stateFileField.SetValue(null, originalValue); + } + } + finally + { + Directory.Delete(tempDir, true); + } + } + + [Fact] + public void Repositories_ReturnsCopy_ThreadSafe() + { + var rm = new RepoManager(); + // Inject state with some repos + SetField(rm, "_loaded", true); + SetField(rm, "_loadedSuccessfully", true); + var state = new RepositoryState + { + Repositories = new() { new() { Id = "r1", Name = "R1" }, new() { Id = "r2", Name = "R2" } } + }; + SetField(rm, "_state", state); + + // Get a snapshot + var repos = rm.Repositories; + Assert.Equal(2, repos.Count); + + // Mutate the underlying state + state.Repositories.RemoveAll(r => r.Id == "r1"); + + // Snapshot should be unaffected (it's a copy) + Assert.Equal(2, repos.Count); + } + + #endregion } diff --git a/PolyPilot.Tests/WorktreeStrategyTests.cs b/PolyPilot.Tests/WorktreeStrategyTests.cs index 362d39b4e5..3389f39e7e 100644 --- a/PolyPilot.Tests/WorktreeStrategyTests.cs +++ b/PolyPilot.Tests/WorktreeStrategyTests.cs @@ -610,4 +610,74 @@ public async Task FullyIsolated_CreatedWorktreeIds_MatchesSessionWorktreeIds() } #endregion + + #region Review Finding: Shared strategy auto-creates worktree when no workingDirectory + + [Fact] + public async Task Shared_WithRepoButNoWorkDir_CreatesSharedWorktree() + { + var rm = new FakeRepoManager(new() { new() { Id = "repo-1", Name = "Repo" } }); + var svc = CreateDemoService(rm); + var preset = MakePreset(2, WorktreeStrategy.Shared); + + // workingDirectory: null and worktreeId: null — should auto-create a shared worktree + var group = await svc.CreateGroupFromPresetAsync(preset, + workingDirectory: null, + repoId: "repo-1"); + + Assert.NotNull(group); + // Exactly 1 shared worktree created (not N per session) + Assert.Single(rm.CreateCalls); + Assert.Contains("shared", rm.CreateCalls[0].BranchName); + // Group should track the created worktree + Assert.Single(group!.CreatedWorktreeIds); + Assert.NotNull(group.WorktreeId); + } + + [Fact] + public async Task Shared_WithExistingWorkDir_CreatesNoWorktree() + { + var rm = new FakeRepoManager(new() { new() { Id = "repo-1", Name = "Repo" } }); + var svc = CreateDemoService(rm); + var preset = MakePreset(2, WorktreeStrategy.Shared); + + // workingDirectory provided — no auto-create needed + var group = await svc.CreateGroupFromPresetAsync(preset, + workingDirectory: "/existing/dir", + repoId: "repo-1"); + + Assert.NotNull(group); + Assert.Empty(rm.CreateCalls); + } + + #endregion + + #region Review Finding: WorktreeId set on AgentSessionInfo (not just SessionMeta) + + [Fact] + public async Task FullyIsolated_WorktreeIdSetOnAgentSessionInfo() + { + var rm = new FakeRepoManager(new() { new() { Id = "repo-1", Name = "Repo" } }); + var svc = CreateDemoService(rm); + var preset = MakePreset(2, WorktreeStrategy.FullyIsolated); + + var group = await svc.CreateGroupFromPresetAsync(preset, + workingDirectory: "/fallback", + repoId: "repo-1"); + + // Check that AgentSessionInfo.WorktreeId is set (not just SessionMeta) + var groupSessionNames = svc.Organization.Sessions + .Where(s => s.GroupId == group!.Id) + .Select(s => s.SessionName) + .ToList(); + + Assert.Equal(3, groupSessionNames.Count); // 1 orch + 2 workers + + var organized = svc.GetOrganizedSessions(); + var groupEntry = organized.FirstOrDefault(g => g.Group.Id == group!.Id); + Assert.NotNull(groupEntry.Group); + Assert.All(groupEntry.Sessions, s => Assert.NotNull(s.WorktreeId)); + } + + #endregion } From 454b6119e90a7f1c6097cd7a4f9378c8717846fa Mon Sep 17 00:00:00 2001 From: Shane Date: Wed, 25 Feb 2026 18:35:22 -0600 Subject: [PATCH 33/33] Address round-2 review: portal cleanup, thread safety, shared strategy Critical: - IAsyncDisposable on SessionListItem: removes portaled overlay on dispose to prevent permanent UI-blocking dark backdrop - Scoped data-dialog-id selector: each instance uses a unique ID instead of global .close-dialog-overlay, preventing cross-instance interference Moderate: - Shared strategy workers use orchWorkDir: fallback chain is now workerWorkDirs[i] ?? orchWorkDir ?? workingDirectory (was skipping orchWorkDir) - Consistent _stateLock on all Add/Remove paths in RepoManager - Git -- separator added to worktree add and branch -D commands - args.RepoId derived from worktree when only WorktreeId is set - Quick-branch input sanitized with Regex (matching CreateSessionForm) Tests: - Shared_WithRepoButNoWorkDir now verifies all sessions share same directory Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- PolyPilot.Tests/WorktreeStrategyTests.cs | 11 +++++ .../Components/Layout/SessionListItem.razor | 28 ++++++++--- .../Components/Layout/SessionSidebar.razor | 13 +++-- .../Services/CopilotService.Organization.cs | 2 +- PolyPilot/Services/RepoManager.cs | 49 ++++++++++++++----- 5 files changed, 78 insertions(+), 25 deletions(-) diff --git a/PolyPilot.Tests/WorktreeStrategyTests.cs b/PolyPilot.Tests/WorktreeStrategyTests.cs index 3389f39e7e..6b38cfa9de 100644 --- a/PolyPilot.Tests/WorktreeStrategyTests.cs +++ b/PolyPilot.Tests/WorktreeStrategyTests.cs @@ -632,6 +632,17 @@ public async Task Shared_WithRepoButNoWorkDir_CreatesSharedWorktree() // Group should track the created worktree Assert.Single(group!.CreatedWorktreeIds); Assert.NotNull(group.WorktreeId); + + // All sessions (orch + workers) should share the same working directory + var organized = svc.GetOrganizedSessions(); + var groupSessions = organized.FirstOrDefault(g => g.Group.Id == group!.Id).Sessions; + Assert.NotNull(groupSessions); + Assert.Equal(3, groupSessions!.Count); // 1 orch + 2 workers + // All sessions should have a non-null working directory (the shared worktree path) + Assert.All(groupSessions, s => Assert.NotNull(s.WorkingDirectory)); + // All should share the same directory + var dirs = groupSessions.Select(s => s.WorkingDirectory).Distinct().ToList(); + Assert.Single(dirs); } [Fact] diff --git a/PolyPilot/Components/Layout/SessionListItem.razor b/PolyPilot/Components/Layout/SessionListItem.razor index e56598bdc4..3214bcbfc3 100644 --- a/PolyPilot/Components/Layout/SessionListItem.razor +++ b/PolyPilot/Components/Layout/SessionListItem.razor @@ -4,6 +4,7 @@ @inject CopilotService CopilotService @inject RepoManager RepoManager @inject IJSRuntime JS +@implements IAsyncDisposable @{ var isPinned = Meta?.IsPinned ?? false; @@ -208,7 +209,7 @@ @if (showCloseConfirm) { -
+
Close "@Session.Name"?
@if (!string.IsNullOrEmpty(Meta?.WorktreeId)) @@ -260,6 +261,9 @@ private bool deleteWorktree; private bool deleteBranch; private ElementReference dialogOverlayRef; + private readonly string _dialogId = Guid.NewGuid().ToString("N")[..8]; + + private string DialogSelector => $"[data-dialog-id='{_dialogId}']"; private async Task ShowCloseConfirm() { @@ -271,15 +275,19 @@ // Portal the overlay to document.body after render so it escapes // overflow:hidden on sidebar/session-list parent containers. await Task.Yield(); - try { await JS.InvokeVoidAsync("eval", "var el=document.querySelector('.close-dialog-overlay');if(el&&el.parentElement!==document.body)document.body.appendChild(el)"); } + try { await JS.InvokeVoidAsync("eval", $"var el=document.querySelector(\"{DialogSelector}\");if(el&&el.parentElement!==document.body)document.body.appendChild(el)"); } catch { /* JS interop may fail during dispose */ } } - private async Task HideCloseConfirm() + private async Task RemovePortaledOverlay() { - // Remove the portaled overlay from document.body before Blazor tries to diff - try { await JS.InvokeVoidAsync("eval", "var el=document.querySelector('.close-dialog-overlay');if(el&&el.parentElement===document.body)el.remove()"); } + try { await JS.InvokeVoidAsync("eval", $"var el=document.querySelector(\"{DialogSelector}\");if(el&&el.parentElement===document.body)el.remove()"); } catch { } + } + + private async Task HideCloseConfirm() + { + await RemovePortaledOverlay(); showCloseConfirm = false; await OnCloseMenu.InvokeAsync(); } @@ -292,9 +300,7 @@ private async Task ConfirmClose() { - // Remove portaled overlay before Blazor diff - try { await JS.InvokeVoidAsync("eval", "var el=document.querySelector('.close-dialog-overlay');if(el&&el.parentElement===document.body)el.remove()"); } - catch { } + await RemovePortaledOverlay(); showCloseConfirm = false; await OnCloseMenu.InvokeAsync(); if (!string.IsNullOrEmpty(Meta?.WorktreeId) && (deleteWorktree || deleteBranch)) @@ -437,4 +443,10 @@ private static string FormatTokenCount(int count) => count >= 1000 ? $"{count / 1000}k" : count.ToString(); + + public async ValueTask DisposeAsync() + { + if (showCloseConfirm) + await RemovePortaledOverlay(); + } } diff --git a/PolyPilot/Components/Layout/SessionSidebar.razor b/PolyPilot/Components/Layout/SessionSidebar.razor index ef2b8415da..732de169c1 100644 --- a/PolyPilot/Components/Layout/SessionSidebar.razor +++ b/PolyPilot/Components/Layout/SessionSidebar.razor @@ -952,9 +952,16 @@ else if (!string.IsNullOrEmpty(args.RepoId) || !string.IsNullOrEmpty(args.WorktreeId)) { - // Repo-based: atomic worktree+session creation + // Repo-based: atomic worktree+session creation. + // When only WorktreeId is set, derive repoId from the worktree. + var repoId = args.RepoId; + if (string.IsNullOrEmpty(repoId) && !string.IsNullOrEmpty(args.WorktreeId)) + { + var wt = RepoManager.Worktrees.FirstOrDefault(w => w.Id == args.WorktreeId); + repoId = wt?.RepoId ?? ""; + } sessionInfo = await CopilotService.CreateSessionWithWorktreeAsync( - repoId: args.RepoId!, + repoId: repoId!, branchName: args.BranchName, prNumber: args.PrNumber, worktreeId: args.WorktreeId, @@ -1051,7 +1058,7 @@ else return; } else - branchName = input; + branchName = System.Text.RegularExpressions.Regex.Replace(input, @"[^a-zA-Z0-9/_.-]", "-").Trim('-'); await CopilotService.CreateSessionWithWorktreeAsync( repoId: repoId, diff --git a/PolyPilot/Services/CopilotService.Organization.cs b/PolyPilot/Services/CopilotService.Organization.cs index d86a8277c9..0a55976f04 100644 --- a/PolyPilot/Services/CopilotService.Organization.cs +++ b/PolyPilot/Services/CopilotService.Organization.cs @@ -1515,7 +1515,7 @@ public string GetEffectiveModel(string sessionName) workerName = $"{teamName}-worker-{i + 1}-{suffix++}"; } var workerModel = preset.WorkerModels[i]; - var workerWorkDir = workerWorkDirs[i] ?? workingDirectory; + var workerWorkDir = workerWorkDirs[i] ?? orchWorkDir ?? workingDirectory; Debug($"[WorktreeStrategy] Worker '{workerName}': wtId={workerWtIds[i] ?? "(none)"}, dir={workerWorkDir ?? "(null)"}"); try { diff --git a/PolyPilot/Services/RepoManager.cs b/PolyPilot/Services/RepoManager.cs index 288263d3e3..d730797b34 100644 --- a/PolyPilot/Services/RepoManager.cs +++ b/PolyPilot/Services/RepoManager.cs @@ -197,7 +197,10 @@ await RunGitAsync(barePath, ct, "config", "remote.origin.fetch", BareClonePath = barePath, AddedAt = DateTime.UtcNow }; - _state.Repositories.Add(repo); + lock (_stateLock) + { + _state.Repositories.Add(repo); + } Save(); OnStateChanged?.Invoke(); return repo; @@ -239,7 +242,7 @@ public virtual async Task CreateWorktreeAsync(string repoId, strin try { - await RunGitAsync(repo.BareClonePath, ct, "worktree", "add", worktreePath, "-b", branchName, baseRef); + await RunGitAsync(repo.BareClonePath, ct, "worktree", "add", worktreePath, "-b", branchName, "--", baseRef); } catch { @@ -257,7 +260,10 @@ public virtual async Task CreateWorktreeAsync(string repoId, strin Path = worktreePath, CreatedAt = DateTime.UtcNow }; - _state.Worktrees.Add(wt); + lock (_stateLock) + { + _state.Worktrees.Add(wt); + } Save(); OnStateChanged?.Invoke(); return wt; @@ -329,7 +335,7 @@ public async Task CreateWorktreeFromPrAsync(string repoId, int prN var worktreeId = Guid.NewGuid().ToString()[..8]; var worktreePath = Path.Combine(WorktreesDir, $"{repoId}-{worktreeId}"); - await RunGitAsync(repo.BareClonePath, ct, "worktree", "add", worktreePath, branchName); + await RunGitAsync(repo.BareClonePath, ct, "worktree", "add", worktreePath, "--", branchName); // Set upstream tracking so push/pull work in the worktree if (headBranch != null) @@ -355,7 +361,10 @@ public async Task CreateWorktreeFromPrAsync(string repoId, int prN Remote = remoteName, CreatedAt = DateTime.UtcNow }; - _state.Worktrees.Add(wt); + lock (_stateLock) + { + _state.Worktrees.Add(wt); + } Save(); OnStateChanged?.Invoke(); return wt; @@ -386,7 +395,7 @@ public async Task RemoveWorktreeAsync(string worktreeId, bool deleteBranch = fal } // Optionally clean up the branch too if (deleteBranch && !string.IsNullOrEmpty(wt.Branch)) - try { await RunGitAsync(repo.BareClonePath, ct, "branch", "-D", wt.Branch); } catch { } + try { await RunGitAsync(repo.BareClonePath, ct, "branch", "-D", "--", wt.Branch); } catch { } } else if (Directory.Exists(wt.Path)) { @@ -409,7 +418,9 @@ public async Task RemoveWorktreeAsync(string worktreeId, bool deleteBranch = fal /// List worktrees for a specific repository. ///
public IEnumerable GetWorktrees(string repoId) - => _state.Worktrees.Where(w => w.RepoId == repoId); + { + lock (_stateLock) return _state.Worktrees.Where(w => w.RepoId == repoId).ToList(); + } /// /// Add a worktree to the in-memory list (for remote mode — tracks server worktrees without running git). @@ -417,8 +428,11 @@ public IEnumerable GetWorktrees(string repoId) public void AddRemoteWorktree(WorktreeInfo wt) { EnsureLoaded(); - if (!_state.Worktrees.Any(w => w.Id == wt.Id)) - _state.Worktrees.Add(wt); + lock (_stateLock) + { + if (!_state.Worktrees.Any(w => w.Id == wt.Id)) + _state.Worktrees.Add(wt); + } } /// @@ -427,8 +441,11 @@ public void AddRemoteWorktree(WorktreeInfo wt) public void AddRemoteRepo(RepositoryInfo repo) { EnsureLoaded(); - if (!_state.Repositories.Any(r => r.Id == repo.Id)) - _state.Repositories.Add(repo); + lock (_stateLock) + { + if (!_state.Repositories.Any(r => r.Id == repo.Id)) + _state.Repositories.Add(repo); + } } /// @@ -437,7 +454,10 @@ public void AddRemoteRepo(RepositoryInfo repo) public void RemoveRemoteWorktree(string worktreeId) { EnsureLoaded(); - _state.Worktrees.RemoveAll(w => w.Id == worktreeId); + lock (_stateLock) + { + _state.Worktrees.RemoveAll(w => w.Id == worktreeId); + } } /// @@ -446,7 +466,10 @@ public void RemoveRemoteWorktree(string worktreeId) public void RemoveRemoteRepo(string repoId) { EnsureLoaded(); - _state.Repositories.RemoveAll(r => r.Id == repoId); + lock (_stateLock) + { + _state.Repositories.RemoveAll(r => r.Id == repoId); + } } ///