Add bulk delete for sessions in the sidebar#210
Merged
dhilgaertner merged 1 commit intomainfrom Apr 24, 2026
Merged
Conversation
Adds a Select-mode toggle to the session sidebar so multiple sessions can be checked and deleted in one confirmation. Mirrors the existing TicketBoardView selection pattern; iterates the existing per-session delete cascade serially so all worktree, branch, terminal, and store cleanup remains intact. Closes #191 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
dgershman
approved these changes
Apr 24, 2026
Collaborator
dgershman
left a comment
There was a problem hiding this comment.
Code & Security Review
Critical Issues
None.
Security Review
Strengths:
- Destructive bulk operation is gated behind a confirmation dialog — users must explicitly confirm before any sessions/worktrees/branches are deleted.
- Main-repo checkouts and protected branches (main, master, develop, etc.) are preserved via the existing
isMainRepoCheckoutguard — the bulk path reuses the same cascade as single-session deletion, so no new bypass vectors are introduced. - Serial iteration through
onDeleteSessionreuses the existing validated deletion pipeline. No new shell/git commands, no new file system operations. - No secrets, no user input passed to shell commands, no injection surface.
Concerns:
- None identified.
Code Quality
Well done:
- Clean separation:
buildBulkMessageis a pure, testable static method on the existingDeleteSessionMessageBuilderenum. 5 new tests cover the key message permutations (no worktrees, singular, real worktrees, mixed, main-only). BulkDeleteSessionsAlertfollows the sameViewModifierpattern asDeleteSessionAlert, keeping the approach consistent.SessionRowgains selection mode cleanly: the existing row content is extracted intorowContent, and the checkbox/HStack wrapper is only added whenisSelectionModeis true. Default parameter values (isSelectionMode: false,selectedSessionIDs: nil) preserve backward compatibility.- Stable deletion order via
sortedSnapshot— processes sessions in their current list order rather than arbitrarySetiteration.
Minor observations (non-blocking):
- If sessions are deleted externally (e.g., via CLI
crow delete-session) between the user tapping "Delete" and the async loop executing, stale IDs become silent no-ops inSessionService.deleteSession. This is harmless but could be mildly confusing. A future enhancement could filterselectedIDsagainstappState.sessionsat execution time — but this is not a bug today. - The
selectedSessionIDsbinding onSessionRowis an optionalBinding<Set<UUID>>?. This works but is slightly unusual — a non-optional binding with a no-op default (e.g.,.constant([])) paired with the existingisSelectionModebool would also work. Current approach is fine.
Summary Table
| Priority | Issue |
|---|---|
| Green | Stale selection IDs become silent no-ops if sessions deleted externally — cosmetic, not a bug |
| Green | Optional binding pattern on SessionRow is functional but slightly unconventional |
Recommendation: Approve. The PR is well-structured, reuses existing deletion infrastructure, includes good test coverage, and introduces no security concerns. The UI additions (toolbar toggle, checkbox column, bulk action bar, confirmation dialog) are clean and consistent with existing patterns.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
SessionService.deleteSessioncascade (worktrees, branches, terminals, hook state, persistence) by iteratingappState.onDeleteSessionserially — noAppStateor service changes.TicketBoardViewselection pattern so the UX is consistent. Single-session deletion via context menu / detail-view button is unchanged.Closes #191
Test plan
make buildsucceedsswift test --package-path Packages/CrowUI— 22/22 passing, including 5 newbuildBulkMessagetestsgit worktree list), feature branches deleted (protected branches preserved), selection mode exits.🤖 Generated with Claude Code