Skip to content

Add bulk delete for sessions in the sidebar#210

Merged
dhilgaertner merged 1 commit intomainfrom
feature/crow-191-mass-delete-items
Apr 24, 2026
Merged

Add bulk delete for sessions in the sidebar#210
dhilgaertner merged 1 commit intomainfrom
feature/crow-191-mass-delete-items

Conversation

@dhilgaertner
Copy link
Copy Markdown
Contributor

@dhilgaertner dhilgaertner commented Apr 24, 2026

Summary

  • Adds a Select toggle to the session sidebar; checked rows reveal a bulk action bar with Cancel / Delete (N) and a single confirmation dialog.
  • Reuses the existing SessionService.deleteSession cascade (worktrees, branches, terminals, hook state, persistence) by iterating appState.onDeleteSession serially — no AppState or service changes.
  • Mirrors the existing TicketBoardView selection pattern so the UX is consistent. Single-session deletion via context menu / detail-view button is unchanged.

Closes #191

Test plan

  • make build succeeds
  • swift test --package-path Packages/CrowUI — 22/22 passing, including 5 new buildBulkMessage tests
  • Click the new ✓ toolbar button in the sidebar; confirm checkboxes appear on rows in Active / Reviews / In Review / Completed sections (and not on Manager / Allow List / Ticket Board / dividers).
  • Select 2-3 sessions across different sections; confirm action bar appears with correct count.
  • Click Delete (N) → confirm bulk alert message reflects worktree/main-checkout counts. Cancel preserves selection.
  • Confirm delete → all selected sessions removed, worktree dirs gone (git worktree list), feature branches deleted (protected branches preserved), selection mode exits.
  • In selection mode, clicking a row body still navigates to the detail view; only checkbox tap toggles selection.
  • Click toolbar Cancel (✗ icon) — checkboxes disappear and selection clears.

🤖 Generated with Claude Code

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>
@dhilgaertner dhilgaertner requested a review from dgershman as a code owner April 24, 2026 22:17
Copy link
Copy Markdown
Collaborator

@dgershman dgershman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 isMainRepoCheckout guard — the bulk path reuses the same cascade as single-session deletion, so no new bypass vectors are introduced.
  • Serial iteration through onDeleteSession reuses 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: buildBulkMessage is a pure, testable static method on the existing DeleteSessionMessageBuilder enum. 5 new tests cover the key message permutations (no worktrees, singular, real worktrees, mixed, main-only).
  • BulkDeleteSessionsAlert follows the same ViewModifier pattern as DeleteSessionAlert, keeping the approach consistent.
  • SessionRow gains selection mode cleanly: the existing row content is extracted into rowContent, and the checkbox/HStack wrapper is only added when isSelectionMode is 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 arbitrary Set iteration.

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 in SessionService.deleteSession. This is harmless but could be mildly confusing. A future enhancement could filter selectedIDs against appState.sessions at execution time — but this is not a bug today.
  • The selectedSessionIDs binding on SessionRow is an optional Binding<Set<UUID>>?. This works but is slightly unusual — a non-optional binding with a no-op default (e.g., .constant([])) paired with the existing isSelectionMode bool 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.

@dhilgaertner dhilgaertner merged commit 1f6ab5a into main Apr 24, 2026
2 checks passed
@dhilgaertner dhilgaertner deleted the feature/crow-191-mass-delete-items branch April 24, 2026 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Mass delete/clear items from the board

2 participants