Run session deletion off the main actor (closes #250)

[dhilgaertner]

Summary

• SessionService.deleteSession now snapshots its work on the main actor, runs git worktree remove/branch -D/worktree prune and the residual FileManager.removeItem inside Task.detached(priority: .utility), then finalizes state back on the main actor — so the UI repaints during the slow part instead of beach-balling.
• Adds AppState.isDeletingSession and AppState.sessionDeletionError. While a delete is in flight the row dims and shows a ProgressView; on success the row goes away; on a fatal cleanup failure the session stays in place with a red exclamationmark.triangle.fill (with hover-text describing the cause) for ~6 s so the user can retry. Per-row and detail-view delete actions, plus the "Mark as In Review" / "Mark as Completed" context-menu items, are disabled while a delete is in flight.
• BulkDeleteSessionsAlert now fans each delete out as its own Task (joined via await task.value) instead of iterating serially, so multi-select delete of N sessions finishes in roughly the time of a single delete.
• The CLI delete-session RPC inherits the same path — no separate code path.

Test plan

• make build is clean
• swift test (root) and swift test in Packages/CrowUI both pass (existing 63 + 22 tests)
• Single delete via row context menu: row dims + spinner appears; sidebar/main view stay interactive (scroll list, switch sessions); row disappears once cleanup completes
• Multi-select delete of 5 and 10 sessions: all selected rows show spinners simultaneously; UI stays responsive; rows disappear in roughly one delete's time, not N×
• Force a failure (rm -rf a worktree directory before pressing delete so git worktree remove fails): row stays, red triangle appears with hover-text, NSLog matches; retrying after the auto-clear succeeds
• CLI: crow delete-session --session <uuid> still completes without UI freeze
• After each scenario, git worktree list and ~/Library/Application Support/Crow/store.json show no leftover state for the deleted IDs

Closes #250

🤖 Generated with Claude Code

[dgershman]

Code & Security Review

Critical Issues

None found.

Security Review

Strengths:

• WorktreeCleanupItem is Sendable — clean data transfer across actor boundaries with no risk of data races on AppState.
• Protected branch guard (SessionWorktree.isProtectedBranch) is preserved on the background path, preventing accidental deletion of main/master.
• Manager session (00000000-...) delete guard is preserved.
• ShellEnvironment.shared.env is used for Process.environment, keeping the environment consistent and avoiding leaking user PATH variations.

Concerns:

• None material. The runShellSync helper constructs Process arguments from trusted internal state (repo paths, branch names) rather than user input, so command injection is not a risk here.

Code Quality

Well done:

• The snapshot-then-detach pattern (WorktreeCleanupItem value type → Task.detached) is the correct way to avoid holding @MainActor-isolated references across an await boundary. Clean separation of concerns.
• The re-entrancy guard (guard appState.isDeletingSession[id] != true) prevents double-delete races.
• performDiskCleanup is nonisolated static — no accidental captures of self or appState. Correct.
• Error auto-clear with the 6-second Task.sleep is a nice UX pattern, and the [weak appState] capture avoids a retain cycle.
• Bulk delete fan-out (Task per session, await t.value to join) is a straightforward improvement over serial iteration.

Minor observations (non-blocking):

1. runShellSync reads pipes after waitUntilExit (SessionService.swift:651-652): calling readDataToEndOfFile() after the process exits is fine for the short outputs expected here (git status messages), but for very large stdout/stderr this can technically deadlock if the pipe buffer fills before the process exits. Not a practical concern for git worktree remove / git branch -D / git worktree prune — mentioning only for awareness.

2. Bulk delete error visibility (DeleteSessionAlert.swift:157-163): when individual deletes in a bulk operation fail, errors are NSLog'd but the per-session error indicators will show on the rows (via sessionDeletionError). The onCompletion() callback (which clears the selection) fires after all tasks join — if some sessions failed and remained, they'll be deselected but show the red triangle. This is reasonable behavior.

3. onCompletion() removed MainActor.run wrapper (DeleteSessionAlert.swift:165): the outer Task { } inherits the main actor from the @MainActor-isolated body context, so after the for t in tasks { await t.value } loop resumes, we're back on the main actor. This is correct.

Summary Table

Priority	Issue
🟢	readDataToEndOfFile after waitUntilExit — fine for these short git commands
🟢	Consider whether bulk-delete selection clearing after partial failure is the desired UX

Recommendation: Approve — the async-delete architecture is sound, the concurrency boundaries are correct, the UI feedback (spinner, error indicator, disabled state) is thorough, and there are no security concerns. Ship it.