Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
bot
added
size:L
Prunty2
changed the title
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| @@ -83,6 +90,7 @@ export function useThreadActions() { | |||
| : null; | |||
| const canDeleteWorktree = orphanedWorktreePath !== null && threadProject !== undefined; | |||
| const shouldDeleteWorktree = | |||
There was a problem hiding this comment.
🟡 Medium hooks/useThreadActions.ts:92
When opts.skipWorktreeConfirm is true, the worktree is never deleted. The expression !opts.skipWorktreeConfirm && canDeleteWorktree && (await api.dialogs.confirm(...)) evaluates to false immediately when skipWorktreeConfirm is set, so shouldDeleteWorktree becomes false and the worktree cleanup is skipped entirely. For batch deletions this leaves orphaned worktrees behind even though the option name suggests only the confirmation dialog should be bypassed. Consider changing the logic to canDeleteWorktree && (opts.skipWorktreeConfirm || (await api.dialogs.confirm(...))) so the worktree is deleted without prompting when the flag is set.
🤖 Copy this AI Prompt to have your agent fix this:
In file apps/web/src/hooks/useThreadActions.ts around line 92:
When `opts.skipWorktreeConfirm` is `true`, the worktree is never deleted. The expression `!opts.skipWorktreeConfirm && canDeleteWorktree && (await api.dialogs.confirm(...))` evaluates to `false` immediately when `skipWorktreeConfirm` is set, so `shouldDeleteWorktree` becomes `false` and the worktree cleanup is skipped entirely. For batch deletions this leaves orphaned worktrees behind even though the option name suggests only the confirmation dialog should be bypassed. Consider changing the logic to `canDeleteWorktree && (opts.skipWorktreeConfirm || (await api.dialogs.confirm(...)))` so the worktree is deleted without prompting when the flag is set.
Evidence trail:
apps/web/src/hooks/useThreadActions.ts lines 92-102 (shouldDeleteWorktree definition with `!opts.skipWorktreeConfirm && canDeleteWorktree && (await api.dialogs.confirm(...))`), lines 150-152 (early return when shouldDeleteWorktree is false), lines 154-159 (worktree deletion via removeWorktreeMutation.mutateAsync only reached if shouldDeleteWorktree is true). Commit: REVIEWED_COMMIT
There was a problem hiding this comment.
Pull request overview
Enables deleting a project even when it still contains chats/threads by introducing a destructive confirmation dialog and a bulk-delete flow that clears related UI state and keeps navigation stable.
Changes:
- Replace the “project not empty” hard stop with an
AlertDialogconfirmation in the sidebar. - Bulk-delete all threads in a project (reusing
deleteThreadwith new options) before dispatchingproject.delete. - Add cross-project fallback-thread selection logic (+ a focused unit test) to keep routing stable after bulk deletion.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| apps/web/src/hooks/useThreadActions.ts | Extends deleteThread with options to suppress navigation and bypass worktree confirmation during bulk deletion. |
| apps/web/src/components/Sidebar.tsx | Adds project deletion confirmation dialog and bulk delete orchestration + fallback navigation. |
| apps/web/src/components/Sidebar.logic.ts | Adds getFallbackThreadIdAfterBulkDelete to pick a remaining non-archived thread after bulk deletion. |
| apps/web/src/components/Sidebar.logic.test.ts | Adds a unit test for the bulk-delete fallback-thread selection behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const canDeleteWorktree = orphanedWorktreePath !== null && threadProject !== undefined; | ||
| const shouldDeleteWorktree = | ||
| !opts.skipWorktreeConfirm && | ||
| canDeleteWorktree && | ||
| (await api.dialogs.confirm( |
There was a problem hiding this comment.
skipWorktreeConfirm currently prevents worktree deletion entirely (because shouldDeleteWorktree becomes false), not just skipping the confirmation UI. If the intent is to delete orphaned worktrees without prompting during bulk deletion, adjust the boolean logic (or rename the option to reflect that it disables deletion). Otherwise, bulk project deletion will leave orphaned worktrees behind with no cleanup path in this flow.
| const projectThreadIds = [...(threadIdsByProjectId[projectId] ?? [])]; | ||
| setPendingProjectDelete({ | ||
| projectId, | ||
| projectName: project.name, | ||
| threadIds: projectThreadIds, | ||
| threadCount: projectThreadIds.length, | ||
| }); |
There was a problem hiding this comment.
PendingProjectDelete snapshots threadIds/threadCount at context-menu time. If threads are created/moved/deleted before the user confirms, this list can become stale (e.g., leaving threads behind or causing project.delete to fail because the project is no longer empty, and the dialog count can be inaccurate). Prefer recomputing the current thread IDs for the project at confirm-time (or store only projectId/projectName and derive the rest).
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1e0acdd. Configure here.
| : null; | ||
| const canDeleteWorktree = orphanedWorktreePath !== null && threadProject !== undefined; | ||
| const shouldDeleteWorktree = | ||
| !opts.skipWorktreeConfirm && |
There was a problem hiding this comment.
skipWorktreeConfirm skips deletion, not just confirmation
Medium Severity
The skipWorktreeConfirm flag short-circuits shouldDeleteWorktree to false, which skips the actual worktree removal entirely — not just the confirmation dialog. When bulk-deleting a project's threads, any orphaned git worktrees are silently left on disk. The option name implies skipping only the prompt while still performing cleanup, but the implementation prevents both.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 1e0acdd. Configure here.
ApprovabilityVerdict: Needs human review 1 blocking correctness issue found. This PR enables a new capability to delete projects along with all their chats, which is a significant behavior change affecting data deletion. Multiple unresolved review comments have identified a bug where the You can customize Macroscope's approvability policy. Learn more. |
Summary
This fixes project removal so a project can be deleted even when it still contains chats.
What changed
Why
Previously, the UI refused to remove a project until every chat inside it had been deleted individually. That made project cleanup unnecessarily tedious and inconsistent with the destructive action the user was already trying to perform.
Validation
bun fmtbun lintbun typecheckExample [After]
Note
Allow deleting projects with chats via a confirmation dialog in the sidebar
AlertDialogconfirmation that shows the project name and number of chats to be deleted.getFallbackThreadIdAfterBulkDeletein Sidebar.logic.ts to select the top non-archived thread after deletion.deleteThreadin useThreadActions.ts withskipWorktreeConfirmandsuppressNavigationoptions used during bulk deletion.Macroscope summarized 1e0acdd.
Note
Medium Risk
Enables bulk deletion of a project’s threads and changes navigation behavior when the active thread/project is removed, which could cause unexpected data loss or routing edge cases if the new flow misfires.
Overview
Projects can now be removed even when they still contain chats: selecting Remove project opens a destructive
AlertDialogand, on confirmation, deletes all threads in that project before dispatchingproject.delete.To keep routing stable, the sidebar computes a cross-project fallback thread via new
getFallbackThreadIdAfterBulkDeleteand navigates to it (or home) if the active thread/draft is affected. Thread deletion is extended withskipWorktreeConfirmandsuppressNavigationoptions to support this bulk-delete flow without per-thread prompts or intermediate redirects, and a focused logic test covers the bulk fallback selection.Reviewed by Cursor Bugbot for commit 1e0acdd. Bugbot is set up for automated code reviews on this repo. Configure here.