feat: add per-project worktree mode setting#2533
Conversation
Add a `worktreeMode` setting (global + per-project override) with three values: "always" (current behavior), "optional" (user chooses per workspace via a toggle in the New Workspace modal), and "disabled" (never create worktrees — open directly in main repo). - Add WorktreeMode type and WORKTREE_MODES constant to local-db schema - Add worktree_mode column to both settings and projects tables - Add getWorktreeMode/setWorktreeMode tRPC procedures for global setting - Add worktreeMode to project update procedure for per-project override - Modify workspace create procedure to respect effective worktree mode - Add Worktree Mode dropdown to global Git & Worktrees settings page - Add per-project Worktree Mode override in project settings - Register GIT_WORKTREE_MODE in settings search - Show worktree toggle in New Workspace modal when mode is "optional" - Hide branch prefix/worktree location settings when mode is "disabled"
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdd configurable worktree modes (always/optional/disabled) across DB, types, TRPC, and UI; prompt per-project worktree choice on open via a new dialog/store; and add optional recycling (move to OS trash + git worktree prune) when closing or deleting projects/workspaces. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as "Renderer UI"
participant TRPC as "Electron TRPC"
participant DB as "Local DB"
participant FS as "Electron Shell (OS)"
participant Git as "Git (repo CLI)"
UI->>TRPC: closeProject(id, deleteWorktrees=true)
TRPC->>DB: query worktrees & workspace records for project id
DB-->>TRPC: list of worktree paths
loop per path
TRPC->>FS: shell.trashItem(path) (best-effort)
FS-->>TRPC: success/failure
end
TRPC->>Git: run `git worktree prune` on main repo path (best-effort)
Git-->>TRPC: prune result
TRPC->>DB: delete workspaceSections/worktrees/workspaces and project record
DB-->>TRPC: deletion ack
TRPC-->>UI: success / error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts`:
- Around line 318-352: The branch value is written to the workspace record but
the working tree in project.mainRepoPath is never switched, so call the same
checkout logic used by openMainRepoWorkspace (or the repository checkout helper
it uses) before touching/setting last active and returning the existing
workspace: after computing branch (from input.branchName or getCurrentBranch)
and before
touchWorkspace(existing.id)/setLastActiveWorkspace(existing.id)/return, ensure
you run the repo checkout on project.mainRepoPath to switch to branch (use the
existing checkout helper invoked by openMainRepoWorkspace to avoid duplicating
logic).
- Around line 354-389: The insert with onConflictDoNothing() can return an empty
array if another request created the workspace, but the code unconditionally
sets wasExisting: false and still emits track("workspace_created"); update the
logic around getMaxProjectChildTabOrder, the
localDb.insert(...).onConflictDoNothing().returning().all() result, and the ws =
workspace[0] ?? getBranchWorkspace(...) branch to detect whether the insert
returned a row (wasExisting = false) or we fell back to getBranchWorkspace
(wasExisting = true); set wasExisting accordingly and only call
track("workspace_created", ...) when the insert actually created the row (i.e.,
when workspace[0] exists).
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/git/components/GitSettings/GitSettings.tsx`:
- Around line 75-76: The Branch Prefix control is still rendered even when
worktree mode is "disabled"; update the GitSettings JSX to hide the Branch
Prefix block whenever effectiveWorktreeMode is "disabled" by reusing the
existing isWorktreeDisabled boolean (derived from effectiveWorktreeMode and
worktreeMode) and gating the Branch Prefix render with the same condition used
for Delete Local Branch and Worktree Location so the prefix UI is not shown in
disabled mode.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 908c4920-aaf2-41af-8923-a7b607b02b81
📒 Files selected for processing (11)
apps/desktop/src/lib/trpc/routers/projects/projects.tsapps/desktop/src/lib/trpc/routers/settings/index.tsapps/desktop/src/lib/trpc/routers/workspaces/procedures/create.tsapps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsxapps/desktop/src/renderer/routes/_authenticated/settings/git/components/GitSettings/GitSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/components/ProjectSettings/ProjectSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/utils/settings-search/settings-search.tspackages/local-db/drizzle/0037_add_worktree_mode.sqlpackages/local-db/drizzle/meta/_journal.jsonpackages/local-db/src/schema/schema.tspackages/local-db/src/schema/zod.ts
| const maxTabOrder = getMaxProjectChildTabOrder(input.projectId); | ||
| const workspace = localDb | ||
| .insert(workspaces) | ||
| .values({ | ||
| projectId: input.projectId, | ||
| type: "branch", | ||
| branch, | ||
| name: input.name ?? branch, | ||
| tabOrder: maxTabOrder + 1, | ||
| }) | ||
| .onConflictDoNothing() | ||
| .returning() | ||
| .all(); | ||
|
|
||
| const ws = workspace[0] ?? getBranchWorkspace(input.projectId); | ||
| if (!ws) { | ||
| throw new Error("Failed to create or find branch workspace"); | ||
| } | ||
|
|
||
| setLastActiveWorkspace(ws.id); | ||
| activateProject(project); | ||
|
|
||
| track("workspace_created", { | ||
| workspace_id: ws.id, | ||
| project_id: project.id, | ||
| type: "branch", | ||
| }); | ||
|
|
||
| return { | ||
| workspace: ws, | ||
| initialCommands: null, | ||
| worktreePath: project.mainRepoPath, | ||
| projectId: project.id, | ||
| isInitializing: false, | ||
| wasExisting: false, | ||
| }; |
There was a problem hiding this comment.
Preserve wasExisting if the insert loses the race.
After onConflictDoNothing(), workspace[0] ?? getBranchWorkspace(input.projectId) can resolve to a row created by another request, but this branch still returns wasExisting: false and emits workspace_created. openMainRepoWorkspace below already handles this by deriving wasExisting from the insert result.
Suggested adjustment
- const workspace = localDb
+ const insertResult = localDb
.insert(workspaces)
.values({
projectId: input.projectId,
type: "branch",
branch,
name: input.name ?? branch,
tabOrder: maxTabOrder + 1,
})
.onConflictDoNothing()
.returning()
.all();
- const ws = workspace[0] ?? getBranchWorkspace(input.projectId);
+ const wasExisting = insertResult.length === 0;
+ const ws =
+ insertResult[0] ?? getBranchWorkspace(input.projectId);
if (!ws) {
throw new Error("Failed to create or find branch workspace");
}
@@
- track("workspace_created", {
- workspace_id: ws.id,
- project_id: project.id,
- type: "branch",
- });
+ if (!wasExisting) {
+ track("workspace_created", {
+ workspace_id: ws.id,
+ project_id: project.id,
+ type: "branch",
+ });
+ }
@@
- wasExisting: false,
+ wasExisting,
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const maxTabOrder = getMaxProjectChildTabOrder(input.projectId); | |
| const workspace = localDb | |
| .insert(workspaces) | |
| .values({ | |
| projectId: input.projectId, | |
| type: "branch", | |
| branch, | |
| name: input.name ?? branch, | |
| tabOrder: maxTabOrder + 1, | |
| }) | |
| .onConflictDoNothing() | |
| .returning() | |
| .all(); | |
| const ws = workspace[0] ?? getBranchWorkspace(input.projectId); | |
| if (!ws) { | |
| throw new Error("Failed to create or find branch workspace"); | |
| } | |
| setLastActiveWorkspace(ws.id); | |
| activateProject(project); | |
| track("workspace_created", { | |
| workspace_id: ws.id, | |
| project_id: project.id, | |
| type: "branch", | |
| }); | |
| return { | |
| workspace: ws, | |
| initialCommands: null, | |
| worktreePath: project.mainRepoPath, | |
| projectId: project.id, | |
| isInitializing: false, | |
| wasExisting: false, | |
| }; | |
| const maxTabOrder = getMaxProjectChildTabOrder(input.projectId); | |
| const insertResult = localDb | |
| .insert(workspaces) | |
| .values({ | |
| projectId: input.projectId, | |
| type: "branch", | |
| branch, | |
| name: input.name ?? branch, | |
| tabOrder: maxTabOrder + 1, | |
| }) | |
| .onConflictDoNothing() | |
| .returning() | |
| .all(); | |
| const wasExisting = insertResult.length === 0; | |
| const ws = | |
| insertResult[0] ?? getBranchWorkspace(input.projectId); | |
| if (!ws) { | |
| throw new Error("Failed to create or find branch workspace"); | |
| } | |
| setLastActiveWorkspace(ws.id); | |
| activateProject(project); | |
| if (!wasExisting) { | |
| track("workspace_created", { | |
| workspace_id: ws.id, | |
| project_id: project.id, | |
| type: "branch", | |
| }); | |
| } | |
| return { | |
| workspace: ws, | |
| initialCommands: null, | |
| worktreePath: project.mainRepoPath, | |
| projectId: project.id, | |
| isInitializing: false, | |
| wasExisting, | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts` around
lines 354 - 389, The insert with onConflictDoNothing() can return an empty array
if another request created the workspace, but the code unconditionally sets
wasExisting: false and still emits track("workspace_created"); update the logic
around getMaxProjectChildTabOrder, the
localDb.insert(...).onConflictDoNothing().returning().all() result, and the ws =
workspace[0] ?? getBranchWorkspace(...) branch to detect whether the insert
returned a row (wasExisting = false) or we fell back to getBranchWorkspace
(wasExisting = true); set wasExisting accordingly and only call
track("workspace_created", ...) when the insert actually created the row (i.e.,
when workspace[0] exists).
| const effectiveWorktreeMode = worktreeMode ?? "always"; | ||
| const isWorktreeDisabled = effectiveWorktreeMode === "disabled"; |
There was a problem hiding this comment.
Hide the Branch Prefix control in disabled mode too.
The new disabled gate is only applied to Delete Local Branch and Worktree Location right now. The Branch Prefix block still renders, even though the disabled create path no longer uses prefixing, so this leaves an irrelevant setting visible and misses the behavior described in the PR.
Suggested fix
- {showBranchPrefix && (
+ {showBranchPrefix && !isWorktreeDisabled && (Also applies to: 223-223, 297-297
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/git/components/GitSettings/GitSettings.tsx`
around lines 75 - 76, The Branch Prefix control is still rendered even when
worktree mode is "disabled"; update the GitSettings JSX to hide the Branch
Prefix block whenever effectiveWorktreeMode is "disabled" by reusing the
existing isWorktreeDisabled boolean (derived from effectiveWorktreeMode and
worktreeMode) and gating the Branch Prefix render with the same condition used
for Delete Local Branch and Worktree Location so the prefix UI is not shown in
disabled mode.
There was a problem hiding this comment.
5 issues found across 11 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/routes/_authenticated/settings/utils/settings-search/settings-search.ts">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/settings/utils/settings-search/settings-search.ts:30">
P2: Per-project Worktree Mode was implemented but not added to the project settings search registry, so project-scope search metadata is incomplete for this new feature.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/settings/git/components/GitSettings/GitSettings.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/settings/git/components/GitSettings/GitSettings.tsx:223">
P2: Branch Prefix is not gated by `isWorktreeDisabled`, so it remains visible when Worktree Mode is disabled, unlike the other worktree-specific settings.</violation>
</file>
<file name="apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx">
<violation number="1" location="apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx:923">
P2: The new Worktree toggle is rendered for optional mode even when creating from a linked PR, but the PR creation path ignores the toggle (createFromPr is called without useWorktree), so the user’s choice is not honored.</violation>
</file>
<file name="apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts">
<violation number="1" location="apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts:320">
P2: `always` worktree policy can be bypassed because `useWorktree: false` is accepted unconditionally, allowing branch workspace creation even when mode resolves to `always`.</violation>
<violation number="2" location="apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts:323">
P1: Disabled/no-worktree create path updates workspace branch metadata without checking out the requested branch, so main-repo workspaces can reference a branch that is not actually checked out.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| BEHAVIOR_RESOURCE_MONITOR: "behavior-resource-monitor", | ||
| BEHAVIOR_OPEN_LINKS_IN_APP: "behavior-open-links-in-app", | ||
|
|
||
| GIT_WORKTREE_MODE: "git-worktree-mode", |
There was a problem hiding this comment.
P2: Per-project Worktree Mode was implemented but not added to the project settings search registry, so project-scope search metadata is incomplete for this new feature.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/settings/utils/settings-search/settings-search.ts, line 30:
<comment>Per-project Worktree Mode was implemented but not added to the project settings search registry, so project-scope search metadata is incomplete for this new feature.</comment>
<file context>
@@ -27,6 +27,7 @@ export const SETTING_ITEM_ID = {
BEHAVIOR_RESOURCE_MONITOR: "behavior-resource-monitor",
BEHAVIOR_OPEN_LINKS_IN_APP: "behavior-open-links-in-app",
+ GIT_WORKTREE_MODE: "git-worktree-mode",
GIT_BRANCH_PREFIX: "git-branch-prefix",
GIT_DELETE_LOCAL_BRANCH: "git-delete-local-branch",
</file context>
| @@ -1,4 +1,4 @@ | |||
| import type { BranchPrefixMode } from "@superset/local-db"; | |||
| import type { BranchPrefixMode, WorktreeMode } from "@superset/local-db"; | |||
There was a problem hiding this comment.
P2: Branch Prefix is not gated by isWorktreeDisabled, so it remains visible when Worktree Mode is disabled, unlike the other worktree-specific settings.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/settings/git/components/GitSettings/GitSettings.tsx, line 223:
<comment>Branch Prefix is not gated by `isWorktreeDisabled`, so it remains visible when Worktree Mode is disabled, unlike the other worktree-specific settings.</comment>
<file context>
@@ -154,7 +186,41 @@ export function GitSettings({ visibleItems }: GitSettingsProps) {
+ </div>
+ )}
+
+ {showDeleteLocalBranch && !isWorktreeDisabled && (
<div className="flex items-center justify-between">
<div className="space-y-0.5">
</file context>
| // open directly in the main repo | ||
| if ( | ||
| effectiveWorktreeMode === "disabled" || | ||
| input.useWorktree === false |
There was a problem hiding this comment.
P2: always worktree policy can be bypassed because useWorktree: false is accepted unconditionally, allowing branch workspace creation even when mode resolves to always.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts, line 320:
<comment>`always` worktree policy can be bypassed because `useWorktree: false` is accepted unconditionally, allowing branch workspace creation even when mode resolves to `always`.</comment>
<file context>
@@ -307,6 +308,87 @@ export const createCreateProcedures = () => {
+ // open directly in the main repo
+ if (
+ effectiveWorktreeMode === "disabled" ||
+ input.useWorktree === false
+ ) {
+ const branch =
</file context>
| input.useWorktree === false | |
| + (effectiveWorktreeMode === "optional" && input.useWorktree === false) |
Overhaul the worktree optional flow:
- Show worktree choice dialog when opening any new project ("With
Worktrees" vs "Without Worktrees")
- Remove "Open without worktree" button from new workspace modal
- Filter worktree-disabled projects from new workspace project picker
- Branch-only projects render as single unified sidebar item with
project name, branch, and diff stats inline (no +, no collapse)
Project settings reorganization:
- Project section (top): worktree on/off toggle + appearance
- Workspace Defaults section (below, hidden when worktrees off):
branch prefix, base branch, worktree location/import
- Indented sections with left border for visual separation
Close project/workspace with worktree cleanup:
- Close Project dialog: 3 buttons (Cancel / Close Project / Recycle
Worktrees) — Recycle moves worktree dirs to macOS Trash
- Close Workspace: new context menu item + dialog with same 3-button
pattern for individual workspaces
- Uses shell.trashItem() for recoverable trash, git worktree prune
for cleanup
Other improvements:
- Prompt textarea placeholder: "What do you want to do? (optional)"
- worktreeMode exposed in getAllGrouped query for sidebar detection
- Auto-init git silently when opening project without worktrees
There was a problem hiding this comment.
7 issues found across 19 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/components/NewWorkspaceModal/components/NewWorkspaceModalContent/NewWorkspaceModalContent.tsx">
<violation number="1" location="apps/desktop/src/renderer/components/NewWorkspaceModal/components/NewWorkspaceModalContent/NewWorkspaceModalContent.tsx:89">
P1: Filtering out `worktreeMode === "disabled"` projects from picker options introduces a behavior regression and can create selected-value/options mismatch in the New Workspace modal.</violation>
</file>
<file name="apps/desktop/src/lib/trpc/routers/workspaces/procedures/query.ts">
<violation number="1" location="apps/desktop/src/lib/trpc/routers/workspaces/procedures/query.ts:155">
P2: `worktreeMode` is typed as `string | null` in `getAllGrouped` response shape, widening the tRPC contract and losing strict `WorktreeMode` union guarantees.</violation>
</file>
<file name="apps/desktop/src/renderer/react-query/projects/useOpenProject.tsx">
<violation number="1" location="apps/desktop/src/renderer/react-query/projects/useOpenProject.tsx:87">
P1: Rejected `updateProject.mutateAsync` can prevent `resolveChoice()` from running, leaving `openNew()` unresolved and potentially producing unhandled promise rejections.</violation>
</file>
<file name="apps/desktop/src/lib/trpc/routers/projects/projects.ts">
<violation number="1" location="apps/desktop/src/lib/trpc/routers/projects/projects.ts:1441">
P2: Worktree trash failures are swallowed, but project/worktree metadata is still deleted and the mutation returns success, causing silent partial cleanup and potential orphaned directories.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/components/ProjectSettings/ProjectSettings.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/components/ProjectSettings/ProjectSettings.tsx:343">
P1: Project worktree override was reduced to a binary toggle, removing `always`/`optional` options and breaking per-project enablement when global mode is `disabled`.</violation>
</file>
<file name="apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx">
<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx:128">
P2: Delete mutation reports the wrong error operation (`close` instead of `delete`), causing misleading failure messages.</violation>
</file>
<file name="apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectSection.tsx">
<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectSection.tsx:89">
P2: Branch-only active state and navigation incorrectly depend on `workspaces[0]`, ignoring section-contained workspaces.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| recentProjects={recentProjects.filter((project) => Boolean(project.id))} | ||
| recentProjects={recentProjects.filter( | ||
| (project) => | ||
| Boolean(project.id) && project.worktreeMode !== "disabled", |
There was a problem hiding this comment.
P1: Filtering out worktreeMode === "disabled" projects from picker options introduces a behavior regression and can create selected-value/options mismatch in the New Workspace modal.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/components/NewWorkspaceModal/components/NewWorkspaceModalContent/NewWorkspaceModalContent.tsx, line 89:
<comment>Filtering out `worktreeMode === "disabled"` projects from picker options introduces a behavior regression and can create selected-value/options mismatch in the New Workspace modal.</comment>
<file context>
@@ -84,7 +84,10 @@ export function NewWorkspaceModalContent({
- recentProjects={recentProjects.filter((project) => Boolean(project.id))}
+ recentProjects={recentProjects.filter(
+ (project) =>
+ Boolean(project.id) && project.worktreeMode !== "disabled",
+ )}
onSelectProject={(selectedProjectId) =>
</file context>
| Boolean(project.id) && project.worktreeMode !== "disabled", | |
| Boolean(project.id), |
| mainRepoPath: string; | ||
| hideImage: boolean; | ||
| iconUrl: string | null; | ||
| worktreeMode: string | null; |
There was a problem hiding this comment.
P2: worktreeMode is typed as string | null in getAllGrouped response shape, widening the tRPC contract and losing strict WorktreeMode union guarantees.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/lib/trpc/routers/workspaces/procedures/query.ts, line 155:
<comment>`worktreeMode` is typed as `string | null` in `getAllGrouped` response shape, widening the tRPC contract and losing strict `WorktreeMode` union guarantees.</comment>
<file context>
@@ -152,6 +152,7 @@ export const createQueryProcedures = () => {
mainRepoPath: string;
hideImage: boolean;
iconUrl: string | null;
+ worktreeMode: string | null;
};
workspaces: WorkspaceItem[];
</file context>
| try { | ||
| await shell.trashItem(wtPath); | ||
| } catch (error) { | ||
| console.error( |
There was a problem hiding this comment.
P2: Worktree trash failures are swallowed, but project/worktree metadata is still deleted and the mutation returns success, causing silent partial cleanup and potential orphaned directories.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/lib/trpc/routers/projects/projects.ts, line 1441:
<comment>Worktree trash failures are swallowed, but project/worktree metadata is still deleted and the mutation returns success, causing silent partial cleanup and potential orphaned directories.</comment>
<file context>
@@ -1399,6 +1404,56 @@ export const createProjectsRouter = (getWindow: () => BrowserWindow | null) => {
+ try {
+ await shell.trashItem(wtPath);
+ } catch (error) {
+ console.error(
+ `[projects/close] Failed to trash worktree ${wtPath}:`,
+ error,
</file context>
| const closeWorkspace = electronTrpc.workspaces.close.useMutation({ | ||
| onSuccess: () => utils.workspaces.getAllGrouped.invalidate(), | ||
| onError: (error) => | ||
| toast.error(`Failed to close workspace: ${error.message}`), |
There was a problem hiding this comment.
P2: Delete mutation reports the wrong error operation (close instead of delete), causing misleading failure messages.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx, line 128:
<comment>Delete mutation reports the wrong error operation (`close` instead of `delete`), causing misleading failure messages.</comment>
<file context>
@@ -120,6 +121,26 @@ export function WorkspaceListItem({
+ const closeWorkspace = electronTrpc.workspaces.close.useMutation({
+ onSuccess: () => utils.workspaces.getAllGrouped.invalidate(),
+ onError: (error) =>
+ toast.error(`Failed to close workspace: ${error.message}`),
+ });
+ const deleteWorkspace = electronTrpc.workspaces.delete.useMutation({
</file context>
| toast.error(`Failed to close workspace: ${error.message}`), | |
| toast.error(`Failed to delete workspace: ${error.message}`), |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
apps/desktop/src/lib/trpc/routers/workspaces/procedures/query.ts (1)
155-156: Consider usingWorktreeModetype instead ofstringfor type safety.The schema defines
worktreeModeas.$type<WorktreeMode>(), but the inline type here usesstring | null. Using the actualWorktreeModetype would provide better type safety and consistency with the database schema.♻️ Proposed fix
Import the type at the top of the file:
import type { WorktreeMode } from "@superset/local-db";Then update the type:
- worktreeMode: string | null; + worktreeMode: WorktreeMode | null;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/workspaces/procedures/query.ts` around lines 155 - 156, The inline result type declares worktreeMode as "string | null" but the schema uses WorktreeMode; import WorktreeMode (import type { WorktreeMode } from "@superset/local-db") at the top of the file and change the field's type from "worktreeMode: string | null" to "worktreeMode: WorktreeMode | null" in the query result type to ensure type safety and consistency with the schema (refer to the worktreeMode field in the query result type).apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx (1)
124-142: Consider passing loading state to prevent double-clicks during mutation.The
CloseWorkspaceDialogaccepts an optionalisLoadingprop (defaulting tofalse) that disables buttons during the operation. Currently, while mutations are in-flight, users can click the dialog buttons multiple times.Suggested improvement
+ const isClosing = closeWorkspace.isPending || deleteWorkspace.isPending; + const handleCloseConfirm = (options: { moveToTrash: boolean }) => { if (options.moveToTrash) { deleteWorkspace.mutate({ id }); } else { closeWorkspace.mutate({ id }); } };Then pass
isLoading={isClosing}toCloseWorkspaceDialog.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx` around lines 124 - 142, The CloseWorkspaceDialog can be clicked multiple times while mutations run; compute an isClosing flag from the two mutations (e.g., const isClosing = closeWorkspace.isLoading || deleteWorkspace.isLoading) and pass it into CloseWorkspaceDialog via its isLoading prop so the dialog buttons are disabled during the in-flight mutation; update references around showCloseDialog, closeWorkspace, deleteWorkspace and handleCloseConfirm to use this isClosing flag.apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectSection.tsx (1)
357-357: Redundant!isBranchOnlycheck.This condition is already inside a
!isBranchOnlyblock (line 346), so the inner check is always true.Suggested fix
- {!isBranchOnly && topLevelChildren.length === 0 && ( + {topLevelChildren.length === 0 && (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectSection.tsx` at line 357, The inner conditional rendering contains a redundant `!isBranchOnly` check inside an outer `!isBranchOnly` block; in ProjectSection.tsx locate the JSX that uses `isBranchOnly` and `topLevelChildren` (the block starting around the outer `!isBranchOnly` check) and remove the duplicate `!isBranchOnly &&` from the inner condition so it simply checks `topLevelChildren.length === 0`; keep the rest of the JSX and behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx`:
- Around line 645-647: The UI currently forces worktrees on/off without letting
users choose when effectiveWorktreeMode === "optional"; add a boolean
useWorktree to the NewWorkspaceModalDraft type/state, render a toggle switch in
PromptGroup's advanced options when effectiveWorktreeMode === "optional" (use
the same UI pattern as other advanced options), bind it to the draft.useWorktree
state so the user can toggle, and ensure the create workspace mutation payload
(the function that submits the draft in PromptGroup or its submit handler)
includes draft.useWorktree instead of implicitly assuming true; update any
defaulting logic so only when effectiveWorktreeMode === "disabled" you set
useWorktree: false automatically.
In
`@apps/desktop/src/renderer/components/WorktreeChoiceDialog/WorktreeChoiceDialog.tsx`:
- Around line 24-71: The dialog currently allows dismissal without a selection
causing onOpenChange(false) to fire but onChoice never called; update
WorktreeChoiceDialog so that AlertDialog cannot be dismissed implicitly (disable
close-on-escape/overlay) or ensure the onOpenChange handler invokes onChoice
with a default (e.g., false) when the dialog is closed without a button press;
locate the AlertDialog usage in WorktreeChoiceDialog and adjust its onOpenChange
callback (or ConnectedWorktreeChoiceDialog wrapper) to call onChoice when
closure is not triggered by the explicit button handlers so
maybePromptWorktreeChoice in useOpenProject.tsx no longer stays pending.
In `@apps/desktop/src/renderer/react-query/projects/useOpenProject.tsx`:
- Around line 75-102: The promise in maybePromptWorktreeChoice can hang because
it only resolves via resolveChoice called from onChoice; update the dialog
dismissal handling so dismissal also invokes onChoice (or otherwise calls
resolveChoice) — e.g., in the dialog component (ConnectedWorktreeChoiceDialog or
WorktreeChoiceDialog) wire its onOpenChange/close handler to call onChoice(true)
when open becomes false (or invoke the store's onChoice callback), ensuring
useWorktreeChoiceDialogStore.open always triggers onChoice on both explicit
selection and dismissal so resolveChoice is called and the loop in
maybePromptWorktreeChoice can continue.
- Around line 87-94: The onChoice handler for the worktrees dialog only persists
the "disabled" case, leaving project.worktreeMode null when the user selects
"With Worktrees" and causing repeated prompts; update the onChoice async handler
(the function passed to onChoice) to call updateProject.mutateAsync with patch:
{ worktreeMode: "always" } when enableWorktrees is true, and then invalidate the
same cache (utils.workspaces.getAllGrouped.invalidate()) so the "always"
selection is persisted and the dialog won't reappear.
In `@apps/desktop/src/renderer/stores/worktree-choice-dialog.ts`:
- Around line 1-32: The dialog close path currently nulls onChoice in
useWorktreeChoiceDialogStore.close() leaving the Promise awaited by
maybePromptWorktreeChoice unresolved; update close() to call the stored onChoice
callback with a default (e.g., onChoice?.(false)) before nulling it so any
pending Promise from maybePromptWorktreeChoice is resolved, and ensure open({
projectName, onChoice }) still sets onChoice as before; reference
useWorktreeChoiceDialogStore, open, close, onChoice, maybePromptWorktreeChoice
and ConnectedWorktreeChoiceDialog when making the change.
---
Nitpick comments:
In `@apps/desktop/src/lib/trpc/routers/workspaces/procedures/query.ts`:
- Around line 155-156: The inline result type declares worktreeMode as "string |
null" but the schema uses WorktreeMode; import WorktreeMode (import type {
WorktreeMode } from "@superset/local-db") at the top of the file and change the
field's type from "worktreeMode: string | null" to "worktreeMode: WorktreeMode |
null" in the query result type to ensure type safety and consistency with the
schema (refer to the worktreeMode field in the query result type).
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectSection.tsx`:
- Line 357: The inner conditional rendering contains a redundant `!isBranchOnly`
check inside an outer `!isBranchOnly` block; in ProjectSection.tsx locate the
JSX that uses `isBranchOnly` and `topLevelChildren` (the block starting around
the outer `!isBranchOnly` check) and remove the duplicate `!isBranchOnly &&`
from the inner condition so it simply checks `topLevelChildren.length === 0`;
keep the rest of the JSX and behavior unchanged.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx`:
- Around line 124-142: The CloseWorkspaceDialog can be clicked multiple times
while mutations run; compute an isClosing flag from the two mutations (e.g.,
const isClosing = closeWorkspace.isLoading || deleteWorkspace.isLoading) and
pass it into CloseWorkspaceDialog via its isLoading prop so the dialog buttons
are disabled during the in-flight mutation; update references around
showCloseDialog, closeWorkspace, deleteWorkspace and handleCloseConfirm to use
this isClosing flag.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4be46712-8ced-4a03-ae15-9bc0262f05b4
📒 Files selected for processing (19)
apps/desktop/src/lib/trpc/routers/projects/projects.tsapps/desktop/src/lib/trpc/routers/workspaces/procedures/query.tsapps/desktop/src/renderer/components/NewWorkspaceModal/components/NewWorkspaceModalContent/NewWorkspaceModalContent.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsxapps/desktop/src/renderer/components/WorktreeChoiceDialog/ConnectedWorktreeChoiceDialog.tsxapps/desktop/src/renderer/components/WorktreeChoiceDialog/WorktreeChoiceDialog.tsxapps/desktop/src/renderer/components/WorktreeChoiceDialog/index.tsapps/desktop/src/renderer/react-query/projects/useOpenProject.tsxapps/desktop/src/renderer/routes/_authenticated/layout.tsxapps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/components/ProjectSettings/ProjectSettings.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/CloseProjectDialog.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectHeader.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectSection.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceContextMenu.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/components/CloseWorkspaceDialog/CloseWorkspaceDialog.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/components/CloseWorkspaceDialog/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebar.tsxapps/desktop/src/renderer/stores/worktree-choice-dialog.ts
✅ Files skipped from review due to trivial changes (2)
- apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/components/CloseWorkspaceDialog/index.ts
- apps/desktop/src/renderer/components/NewWorkspaceModal/components/NewWorkspaceModalContent/NewWorkspaceModalContent.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/lib/trpc/routers/projects/projects.ts
| ...(effectiveWorktreeMode === "disabled" && { | ||
| useWorktree: false, | ||
| }), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if the "optional" mode toggle UI is implemented elsewhere in NewWorkspaceModal
rg -n "optional|useWorktree|worktreeMode" --type=tsx apps/desktop/src/renderer/components/NewWorkspaceModal/ -C 3
# Also check for any toggle component that might be conditionally rendered
ast-grep --pattern 'effectiveWorktreeMode === "optional"'Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
#!/bin/bash
# Check if the "optional" mode toggle UI is implemented
find apps/desktop/src/renderer/components/NewWorkspaceModal -name "*.tsx" -o -name "*.ts" | head -20Repository: superset-sh/superset
Length of output: 1429
🏁 Script executed:
#!/bin/bash
# Search for worktreeMode and optional handling in the files
rg -n "optional|useWorktree|worktreeMode" apps/desktop/src/renderer/components/NewWorkspaceModal/ -C 3Repository: superset-sh/superset
Length of output: 4883
🏁 Script executed:
#!/bin/bash
# Read the PromptGroup.tsx file to see the context around lines 645-647
wc -l apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsxRepository: superset-sh/superset
Length of output: 161
🏁 Script executed:
#!/bin/bash
# Read the context around lines 645-647 in PromptGroup.tsx
sed -n '630,670p' apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx | cat -nRepository: superset-sh/superset
Length of output: 1388
🏁 Script executed:
#!/bin/bash
# Search for any state related to worktree toggle or useWorktree variable assignments
rg -n "useState|useWorktree.*=" apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx | head -30Repository: superset-sh/superset
Length of output: 429
🏁 Script executed:
#!/bin/bash
# Check if there's a toggle component for worktree mode selection
rg -n "Toggle|Checkbox|Switch|optional" apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsxRepository: superset-sh/superset
Length of output: 170
🏁 Script executed:
#!/bin/bash
# Check the PromptGroupAdvancedOptions component which might have the worktree toggle
cat -n apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/components/PromptGroupAdvancedOptions/PromptGroupAdvancedOptions.tsxRepository: superset-sh/superset
Length of output: 8443
🏁 Script executed:
#!/bin/bash
# Check for any "optional" worktree mode handling throughout the entire NewWorkspaceModal
rg -n "optional" apps/desktop/src/renderer/components/NewWorkspaceModal/ -A 5 -B 5Repository: superset-sh/superset
Length of output: 2991
🏁 Script executed:
#!/bin/bash
# Check the full useState declarations to understand what state is tracked
sed -n '40,500p' apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx | grep -n "useState\|const \[" | head -20Repository: superset-sh/superset
Length of output: 428
🏁 Script executed:
#!/bin/bash
# Check the draft context to see if worktree choice is stored there
cat -n apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModalDraftContext.tsx | head -100Repository: superset-sh/superset
Length of output: 3420
🏁 Script executed:
#!/antml:bin/bash
# Search for the mutation definition to understand what useWorktree expects
rg -n "createWorkspace|useWorktree" apps/desktop/src/renderer/components/NewWorkspaceModal/ -B 3 -A 3Repository: superset-sh/superset
Length of output: 8755
🏁 Script executed:
#!/bin/bash
# Check if there's any documentation or PR info about worktree modes
rg -n "worktreeMode.*optional|optional.*worktree" --type=md --type=txt apps/desktop/Repository: superset-sh/superset
Length of output: 398
🏁 Script executed:
#!/bin/bash
# Search for the useCreateWorkspace mutation definition to understand what happens when useWorktree is not passed
fd -t f "useCreateWorkspace" apps/desktop/src/renderer/react-query/workspaces/ | head -5Repository: superset-sh/superset
Length of output: 134
🏁 Script executed:
#!/bin/bash
# Read the useCreateWorkspace hook to understand the mutation schema
cat apps/desktop/src/renderer/react-query/workspaces/useCreateWorkspace.tsRepository: superset-sh/superset
Length of output: 4905
🏁 Script executed:
#!/bin/bash
# Check if there are any tests or comments about worktree mode "optional" behavior
rg -n "optional" apps/desktop/src/renderer/components/NewWorkspaceModal/ -B 2 -A 2Repository: superset-sh/superset
Length of output: 1442
🏁 Script executed:
#!/bin/bash
# Find the tRPC workspaces router to understand the mutation schema
find apps/desktop -name "*.ts" -o -name "*.tsx" | xargs grep -l "workspaces.*create.*procedure\|workspaces.*create.*router" | head -5Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Search for worktree-related code in the backend/shared types
rg -n "worktreeMode.*optional\|optional.*worktree" apps/desktop/ --type=ts --type=tsx -B 5 -A 5Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
#!/antml/bash
# Check the trpc-electron router definition
find apps -name "*router*" -o -name "*trpc*" | grep -E "workspaces|create" | head -10Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Search for where the workspaces.create mutation is defined
rg -n "workspaces\.create\|\.create\(" apps/desktop/src --type ts | grep -i router | head -10Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Look for trpc router or workspace routes
find apps/desktop -type f -name "*.ts" | xargs grep -l "workspaces.*create" | head -10Repository: superset-sh/superset
Length of output: 740
🏁 Script executed:
#!/bin/bash
# Search for the electron trpc definition
rg -n "electronTrpc\|trpc-electron" apps/desktop/src -l | head -10Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Read the create procedure to understand what parameters it accepts
cat apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.tsRepository: superset-sh/superset
Length of output: 29567
🏁 Script executed:
#!/bin/bash
# Search for any recent changes or TODOs related to optional worktree mode
rg -n "TODO|FIXME|optional.*worktree\|worktree.*optional" apps/desktop/src/renderer/components/NewWorkspaceModal/ -B 2 -A 2Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Check if there's an "always" mode to understand the pattern
rg -n "always\|optional\|disabled" apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts | head -20Repository: superset-sh/superset
Length of output: 46
Add worktree toggle UI for "optional" mode.
When effectiveWorktreeMode === "optional", the modal should display a toggle allowing the user to choose whether to create a worktree. Currently, "optional" mode falls through without presenting this choice to the user—worktrees are created by default without user control.
Implement:
- Add a
useWorktreefield toNewWorkspaceModalDraftto track the user's choice - Render a toggle in the advanced options when mode is "optional"
- Pass the user's choice to the mutation payload
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx`
around lines 645 - 647, The UI currently forces worktrees on/off without letting
users choose when effectiveWorktreeMode === "optional"; add a boolean
useWorktree to the NewWorkspaceModalDraft type/state, render a toggle switch in
PromptGroup's advanced options when effectiveWorktreeMode === "optional" (use
the same UI pattern as other advanced options), bind it to the draft.useWorktree
state so the user can toggle, and ensure the create workspace mutation payload
(the function that submits the draft in PromptGroup or its submit handler)
includes draft.useWorktree instead of implicitly assuming true; update any
defaulting logic so only when effectiveWorktreeMode === "disabled" you set
useWorktree: false automatically.
| return ( | ||
| <AlertDialog open={open} onOpenChange={onOpenChange}> | ||
| <AlertDialogContent className="max-w-[420px] gap-0 p-0"> | ||
| <AlertDialogHeader className="px-4 pt-4 pb-3"> | ||
| <AlertDialogTitle className="font-medium"> | ||
| Enable worktrees for "{projectName}"? | ||
| </AlertDialogTitle> | ||
| <AlertDialogDescription asChild> | ||
| <div className="text-muted-foreground space-y-1.5"> | ||
| <span className="block text-xs"> | ||
| <strong>With Worktrees</strong> — each workspace gets its own | ||
| isolated copy of the repo. You can run multiple agents in | ||
| parallel without conflicts. | ||
| </span> | ||
| <span className="block text-xs"> | ||
| <strong>Without Worktrees</strong> — work directly in the | ||
| project folder. One workspace, no copies. | ||
| </span> | ||
| </div> | ||
| </AlertDialogDescription> | ||
| </AlertDialogHeader> | ||
|
|
||
| <AlertDialogFooter className="px-4 pb-4 pt-2 flex-row justify-end gap-2"> | ||
| <Button | ||
| variant="outline" | ||
| size="sm" | ||
| className="h-7 px-3 text-xs" | ||
| onClick={() => { | ||
| onOpenChange(false); | ||
| onChoice(false); | ||
| }} | ||
| > | ||
| Without Worktrees | ||
| </Button> | ||
| <Button | ||
| size="sm" | ||
| className="h-7 px-3 text-xs" | ||
| onClick={() => { | ||
| onOpenChange(false); | ||
| onChoice(true); | ||
| }} | ||
| > | ||
| With Worktrees | ||
| </Button> | ||
| </AlertDialogFooter> | ||
| </AlertDialogContent> | ||
| </AlertDialog> | ||
| ); |
There was a problem hiding this comment.
Dialog allows dismissal without making a selection, which may cause issues upstream.
The AlertDialog can be dismissed via Escape key or clicking outside without triggering either button. When this happens, onOpenChange(false) fires but onChoice is never called. This leaves the Promise in useOpenProject.tsx's maybePromptWorktreeChoice pending indefinitely, blocking subsequent project opens.
Consider preventing unintentional dismissal:
Suggested fix
- <AlertDialog open={open} onOpenChange={onOpenChange}>
+ <AlertDialog open={open} onOpenChange={(isOpen) => {
+ // Only allow closing via button clicks, not escape/overlay
+ if (isOpen) onOpenChange(isOpen);
+ }}>Or treat dismissal as a default choice in ConnectedWorktreeChoiceDialog.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return ( | |
| <AlertDialog open={open} onOpenChange={onOpenChange}> | |
| <AlertDialogContent className="max-w-[420px] gap-0 p-0"> | |
| <AlertDialogHeader className="px-4 pt-4 pb-3"> | |
| <AlertDialogTitle className="font-medium"> | |
| Enable worktrees for "{projectName}"? | |
| </AlertDialogTitle> | |
| <AlertDialogDescription asChild> | |
| <div className="text-muted-foreground space-y-1.5"> | |
| <span className="block text-xs"> | |
| <strong>With Worktrees</strong> — each workspace gets its own | |
| isolated copy of the repo. You can run multiple agents in | |
| parallel without conflicts. | |
| </span> | |
| <span className="block text-xs"> | |
| <strong>Without Worktrees</strong> — work directly in the | |
| project folder. One workspace, no copies. | |
| </span> | |
| </div> | |
| </AlertDialogDescription> | |
| </AlertDialogHeader> | |
| <AlertDialogFooter className="px-4 pb-4 pt-2 flex-row justify-end gap-2"> | |
| <Button | |
| variant="outline" | |
| size="sm" | |
| className="h-7 px-3 text-xs" | |
| onClick={() => { | |
| onOpenChange(false); | |
| onChoice(false); | |
| }} | |
| > | |
| Without Worktrees | |
| </Button> | |
| <Button | |
| size="sm" | |
| className="h-7 px-3 text-xs" | |
| onClick={() => { | |
| onOpenChange(false); | |
| onChoice(true); | |
| }} | |
| > | |
| With Worktrees | |
| </Button> | |
| </AlertDialogFooter> | |
| </AlertDialogContent> | |
| </AlertDialog> | |
| ); | |
| return ( | |
| <AlertDialog open={open} onOpenChange={(isOpen) => { | |
| // Only allow closing via button clicks, not escape/overlay | |
| if (isOpen) onOpenChange(isOpen); | |
| }}> | |
| <AlertDialogContent className="max-w-[420px] gap-0 p-0"> | |
| <AlertDialogHeader className="px-4 pt-4 pb-3"> | |
| <AlertDialogTitle className="font-medium"> | |
| Enable worktrees for "{projectName}"? | |
| </AlertDialogTitle> | |
| <AlertDialogDescription asChild> | |
| <div className="text-muted-foreground space-y-1.5"> | |
| <span className="block text-xs"> | |
| <strong>With Worktrees</strong> — each workspace gets its own | |
| isolated copy of the repo. You can run multiple agents in | |
| parallel without conflicts. | |
| </span> | |
| <span className="block text-xs"> | |
| <strong>Without Worktrees</strong> — work directly in the | |
| project folder. One workspace, no copies. | |
| </span> | |
| </div> | |
| </AlertDialogDescription> | |
| </AlertDialogHeader> | |
| <AlertDialogFooter className="px-4 pb-4 pt-2 flex-row justify-end gap-2"> | |
| <Button | |
| variant="outline" | |
| size="sm" | |
| className="h-7 px-3 text-xs" | |
| onClick={() => { | |
| onOpenChange(false); | |
| onChoice(false); | |
| }} | |
| > | |
| Without Worktrees | |
| </Button> | |
| <Button | |
| size="sm" | |
| className="h-7 px-3 text-xs" | |
| onClick={() => { | |
| onOpenChange(false); | |
| onChoice(true); | |
| }} | |
| > | |
| With Worktrees | |
| </Button> | |
| </AlertDialogFooter> | |
| </AlertDialogContent> | |
| </AlertDialog> | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/components/WorktreeChoiceDialog/WorktreeChoiceDialog.tsx`
around lines 24 - 71, The dialog currently allows dismissal without a selection
causing onOpenChange(false) to fire but onChoice never called; update
WorktreeChoiceDialog so that AlertDialog cannot be dismissed implicitly (disable
close-on-escape/overlay) or ensure the onOpenChange handler invokes onChoice
with a default (e.g., false) when the dialog is closed without a button press;
locate the AlertDialog usage in WorktreeChoiceDialog and adjust its onOpenChange
callback (or ConnectedWorktreeChoiceDialog wrapper) to call onChoice when
closure is not triggered by the explicit button handlers so
maybePromptWorktreeChoice in useOpenProject.tsx no longer stays pending.
| /** Show worktree choice dialog for each newly opened project. */ | ||
| const maybePromptWorktreeChoice = useCallback( | ||
| async (newProjects: Project[]): Promise<void> => { | ||
| if (newProjects.length === 0) return; | ||
|
|
||
| for (const project of newProjects) { | ||
| // Skip projects that already have a worktreeMode set | ||
| if (project.worktreeMode) continue; | ||
|
|
||
| await new Promise<void>((resolveChoice) => { | ||
| useWorktreeChoiceDialogStore.getState().open({ | ||
| projectName: project.name, | ||
| onChoice: async (enableWorktrees) => { | ||
| if (!enableWorktrees) { | ||
| await updateProject.mutateAsync({ | ||
| id: project.id, | ||
| patch: { worktreeMode: "disabled" }, | ||
| }); | ||
| await utils.workspaces.getAllGrouped.invalidate(); | ||
| } | ||
| resolveChoice(); | ||
| }, | ||
| }); | ||
| }); | ||
| } | ||
| }, | ||
| [updateProject, utils], | ||
| ); |
There was a problem hiding this comment.
Dialog dismissal without selection will leave the Promise pending indefinitely.
The await new Promise<void> at line 84 only resolves when resolveChoice() is called inside onChoice. If the user dismisses the dialog (Escape/overlay click) without clicking a button, onChoice is never invoked, and this Promise never resolves. This blocks the entire openNew flow for all subsequent projects in the iteration.
Suggested fix in the store or connected component
Add dismissal handling in ConnectedWorktreeChoiceDialog:
onOpenChange={(open) => {
if (!open) {
// Treat dismissal as "enable worktrees" (default)
onChoice?.(true);
close();
}
}}Or prevent dismissal entirely in WorktreeChoiceDialog.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/react-query/projects/useOpenProject.tsx` around
lines 75 - 102, The promise in maybePromptWorktreeChoice can hang because it
only resolves via resolveChoice called from onChoice; update the dialog
dismissal handling so dismissal also invokes onChoice (or otherwise calls
resolveChoice) — e.g., in the dialog component (ConnectedWorktreeChoiceDialog or
WorktreeChoiceDialog) wire its onOpenChange/close handler to call onChoice(true)
when open becomes false (or invoke the store's onChoice callback), ensuring
useWorktreeChoiceDialogStore.open always triggers onChoice on both explicit
selection and dismissal so resolveChoice is called and the loop in
maybePromptWorktreeChoice can continue.
| onChoice: async (enableWorktrees) => { | ||
| if (!enableWorktrees) { | ||
| await updateProject.mutateAsync({ | ||
| id: project.id, | ||
| patch: { worktreeMode: "disabled" }, | ||
| }); | ||
| await utils.workspaces.getAllGrouped.invalidate(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how worktreeMode null is handled in workspace creation and sidebar
rg -n "worktreeMode" --type ts -C 3 | head -100Repository: superset-sh/superset
Length of output: 11092
🏁 Script executed:
# Check the dialog store and how it handles the choice
rg -n "worktreeChoiceDialogStore\|useWorktreeChoiceDialog" --type ts -B 2 -A 10 | head -80Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Check if there's any logic that treats null differently from "always"
rg -n "project\.worktreeMode" --type tsx -B 1 -A 1 | grep -E "(project\.worktreeMode|===|!==)" | head -40Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
# Check the dialog store and how it handles the choice
rg -n "worktreeChoiceDialogStore" --type ts -B 2 -A 10Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Check broader worktreeMode usage with correct file type
rg -n "worktreeMode" --type ts | grep -E "(===|!==|??|null)"Repository: superset-sh/superset
Length of output: 3334
🏁 Script executed:
# Check if projects are created with null worktreeMode
rg -n "worktreeMode.*null\|null.*worktreeMode" --type tsRepository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Check what WORKTREE_MODES contains
rg -n "WORKTREE_MODES" --type ts -A 3Repository: superset-sh/superset
Length of output: 1595
🏁 Script executed:
# Check the WorktreeMode type definition
rg -n "type WorktreeMode\|enum WorktreeMode" --type ts -A 3Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Check if there's explicit "always" being set anywhere
rg -n "worktreeMode.*always\|always.*worktreeMode" --type tsRepository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Look at the actual choice dialog component to understand intent
rg -n "open.*worktree\|worktreeChoice.*open" --type ts -B 3 -A 5Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Check the full useOpenProject file context
cat -n apps/desktop/src/renderer/react-query/projects/useOpenProject.tsx | sed -n '75,100p'Repository: superset-sh/superset
Length of output: 1057
🏁 Script executed:
# Check if there's any comment or TODO about this asymmetry
rg -n "worktree.*dialog\|dialog.*worktree" --type ts -B 2 -A 2 | head -50Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Look for any tests or examples showing expected behavior
fd "useOpenProject" --type tsRepository: superset-sh/superset
Length of output: 233
🏁 Script executed:
# Check the store implementation to see what happens on choice
rg -n "useWorktreeChoiceDialogStore" --type ts -A 15 | head -60Repository: superset-sh/superset
Length of output: 5794
🏁 Script executed:
# Check the actual dialog component to understand user options
cat -n apps/desktop/src/renderer/components/WorktreeChoiceDialog/WorktreeChoiceDialog.tsxRepository: superset-sh/superset
Length of output: 2479
🏁 Script executed:
# Check if there's any history or PR context about this feature
rg -n "Show worktree choice dialog for each newly opened" --type ts -B 5 -A 20Repository: superset-sh/superset
Length of output: 2677
Persist "always" mode explicitly when user selects "With Worktrees".
When the user chooses "With Worktrees" on line 87, worktreeMode remains null because the code only handles the enableWorktrees === false case (line 88). Since line 82's check uses truthiness (if (project.worktreeMode)), the dialog will show again on every subsequent open until the user explicitly selects "Without Worktrees" to set it to "disabled".
While null is treated as "always" elsewhere in the codebase, the asymmetric persistence means users will be re-prompted repeatedly. Consider explicitly persisting "always" when the user chooses "With Worktrees":
Suggested fix
onChoice: async (enableWorktrees) => {
- if (!enableWorktrees) {
+ if (enableWorktrees) {
+ await updateProject.mutateAsync({
+ id: project.id,
+ patch: { worktreeMode: "always" },
+ });
+ } else {
await updateProject.mutateAsync({
id: project.id,
patch: { worktreeMode: "disabled" },
});
- await utils.workspaces.getAllGrouped.invalidate();
}
+ await utils.workspaces.getAllGrouped.invalidate();
resolveChoice();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| onChoice: async (enableWorktrees) => { | |
| if (!enableWorktrees) { | |
| await updateProject.mutateAsync({ | |
| id: project.id, | |
| patch: { worktreeMode: "disabled" }, | |
| }); | |
| await utils.workspaces.getAllGrouped.invalidate(); | |
| } | |
| onChoice: async (enableWorktrees) => { | |
| if (enableWorktrees) { | |
| await updateProject.mutateAsync({ | |
| id: project.id, | |
| patch: { worktreeMode: "always" }, | |
| }); | |
| } else { | |
| await updateProject.mutateAsync({ | |
| id: project.id, | |
| patch: { worktreeMode: "disabled" }, | |
| }); | |
| } | |
| await utils.workspaces.getAllGrouped.invalidate(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/react-query/projects/useOpenProject.tsx` around
lines 87 - 94, The onChoice handler for the worktrees dialog only persists the
"disabled" case, leaving project.worktreeMode null when the user selects "With
Worktrees" and causing repeated prompts; update the onChoice async handler (the
function passed to onChoice) to call updateProject.mutateAsync with patch: {
worktreeMode: "always" } when enableWorktrees is true, and then invalidate the
same cache (utils.workspaces.getAllGrouped.invalidate()) so the "always"
selection is persisted and the dialog won't reappear.
| import { create } from "zustand"; | ||
| import { devtools } from "zustand/middleware"; | ||
|
|
||
| interface WorktreeChoiceDialogState { | ||
| isOpen: boolean; | ||
| projectName: string; | ||
| onChoice: ((enableWorktrees: boolean) => void) | null; | ||
| open: (params: { | ||
| projectName: string; | ||
| onChoice: (enableWorktrees: boolean) => void; | ||
| }) => void; | ||
| close: () => void; | ||
| } | ||
|
|
||
| export const useWorktreeChoiceDialogStore = create<WorktreeChoiceDialogState>()( | ||
| devtools( | ||
| (set) => ({ | ||
| isOpen: false, | ||
| projectName: "", | ||
| onChoice: null, | ||
|
|
||
| open: ({ projectName, onChoice }) => { | ||
| set({ isOpen: true, projectName, onChoice }); | ||
| }, | ||
|
|
||
| close: () => { | ||
| set({ isOpen: false, projectName: "", onChoice: null }); | ||
| }, | ||
| }), | ||
| { name: "WorktreeChoiceDialogStore" }, | ||
| ), | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how ConnectedWorktreeChoiceDialog handles the onOpenChange callback
ast-grep --pattern $'function ConnectedWorktreeChoiceDialog() {
$$$
}'Repository: superset-sh/superset
Length of output: 2140
🏁 Script executed:
#!/bin/bash
# Search for useOpenProject and maybePromptWorktreeChoice to see how onChoice callback is used
rg "maybePromptWorktreeChoice|useOpenProject" --type ts --type tsx -A 5 -B 2Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
#!/bin/bash
# Check how the store's onChoice is awaited and if there's a default behavior for dismissal
rg "useWorktreeChoiceDialogStore" --type ts --type tsx -A 10 -B 2Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
#!/bin/bash
# Look for any Promise.race or timeout handling around the worktree choice
rg "Promise" apps/desktop/src/renderer --type ts --type tsx | grep -i "worktree\|choice" -A 2 -B 2Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
#!/bin/bash
# Find files related to useOpenProject or worktree choice
fd "useOpenProject|maybePromptWorktreeChoice|WorktreeChoice" apps/desktop/src/renderer --type fRepository: superset-sh/superset
Length of output: 304
🏁 Script executed:
#!/bin/bash
# Search for maybePromptWorktreeChoice without type constraint
rg "maybePromptWorktreeChoice" apps/desktop/src/renderer -A 10 -B 2Repository: superset-sh/superset
Length of output: 4326
🏁 Script executed:
#!/bin/bash
# Search for how useWorktreeChoiceDialogStore is used
rg "useWorktreeChoiceDialogStore" apps/desktop/src/renderer -A 15 -B 2Repository: superset-sh/superset
Length of output: 7511
Store structure is sound, but dismissing the dialog without a selection leaves the Promise unresolved.
The store correctly manages dialog state, but a Promise in maybePromptWorktreeChoice awaits the onChoice callback. When the user dismisses the dialog via Escape or clicking outside, ConnectedWorktreeChoiceDialog calls close(), which nulls onChoice without invoking it. This leaves the Promise hanging indefinitely.
Resolve this by either:
- Calling
onChoicewith a default value inclose()(e.g.,onChoice?.(false)) - Preventing dismissal via escape key and outside click by making the dialog modal
- Adding an
onDismisscallback to handle cancellation separately
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/stores/worktree-choice-dialog.ts` around lines 1 -
32, The dialog close path currently nulls onChoice in
useWorktreeChoiceDialogStore.close() leaving the Promise awaited by
maybePromptWorktreeChoice unresolved; update close() to call the stored onChoice
callback with a default (e.g., onChoice?.(false)) before nulling it so any
pending Promise from maybePromptWorktreeChoice is resolved, and ensure open({
projectName, onChoice }) still sets onChoice as before; reference
useWorktreeChoiceDialogStore, open, close, onChoice, maybePromptWorktreeChoice
and ConnectedWorktreeChoiceDialog when making the change.
…ct open - Add `trash` option to workspaces.delete procedure — uses shell.trashItem() + git worktree prune instead of permanent delete - "Disable & Recycle" in project settings now sends worktrees to Trash - "Recycle Worktree" in workspace close dialog also uses Trash - Invalidate getAllGrouped + getRecents after worktree choice dialog so new projects appear in the sidebar immediately - Capitalize "Worktrees" in dialog titles, use "Enable"/"Disable" button labels instead of "With/Without Worktrees"
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/components/ProjectSettings/ProjectSettings.tsx (1)
787-808: Dialog buttons lack pending state during bulk operation.The "Disable" and "Disable & Recycle" buttons close the dialog immediately and call
handleDisableWorktrees, but there's no pending/loading state to prevent double-clicks or indicate the operation is in progress. Consider:
- Adding a local
isPendingstate that's set duringhandleDisableWorktrees- Keeping the dialog open with disabled buttons until the operation completes
- Or showing a toast indicating the operation has started
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/settings/project/`$projectId/components/ProjectSettings/ProjectSettings.tsx around lines 787 - 808, The dialog buttons currently close the dialog immediately and call handleDisableWorktrees without any pending state; add a local isPending state (e.g., const [isPending, setIsPending] = useState(false)) and update the onClick handlers for the "Disable" and "Disable & Recycle" buttons to set isPending=true before calling handleDisableWorktrees, keep the dialog open while isPending is true (do not call setShowDisableWorktreeDialog(false) until the operation resolves), disable the buttons and show a spinner or pending style while isPending is true, and ensure handleDisableWorktrees clears isPending and only then closes the dialog on success or re-enables buttons on failure.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts`:
- Around line 256-268: Remove the redundant dynamic import of existsSync (use
the top-level existsSync already imported) and wrap the trash-path operations
(shell.trashItem and the subsequent getSimpleGitWithShellPath +
git.raw(["worktree","prune"])) in a try/catch; on error log the failure (using
the existing logger) and abort/throw a TRPC-friendly error so the workspace
record is not deleted, mirroring the non-trash path’s graceful failure handling.
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/project/`$projectId/components/ProjectSettings/ProjectSettings.tsx:
- Around line 247-268: The handleDisableWorktrees function currently uses
updateProject.mutate (non-awaiting) and only console.errors for individual
workspace failures; change updateProject.mutate to await
updateProject.mutateAsync({ id: projectId, patch: { worktreeMode: "disabled" }
}) so the final project update is performed atomically after workspace
operations, and capture/aggregate errors from deleteWorkspace.mutateAsync and
closeWorkspaceMutation.mutateAsync into a list; after all attempts, show a
user-facing toast (success/on partial failure/error) summarizing which workspace
IDs failed and only mark worktrees disabled on successful mutateAsync
completion, handling and surfacing any updateProject.mutateAsync error to the
user as well.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/project/`$projectId/components/ProjectSettings/ProjectSettings.tsx:
- Around line 787-808: The dialog buttons currently close the dialog immediately
and call handleDisableWorktrees without any pending state; add a local isPending
state (e.g., const [isPending, setIsPending] = useState(false)) and update the
onClick handlers for the "Disable" and "Disable & Recycle" buttons to set
isPending=true before calling handleDisableWorktrees, keep the dialog open while
isPending is true (do not call setShowDisableWorktreeDialog(false) until the
operation resolves), disable the buttons and show a spinner or pending style
while isPending is true, and ensure handleDisableWorktrees clears isPending and
only then closes the dialog on success or re-enables buttons on failure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: aa561a11-95b7-4844-98ab-b3499f6e9c79
📒 Files selected for processing (5)
apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.tsapps/desktop/src/renderer/components/WorktreeChoiceDialog/WorktreeChoiceDialog.tsxapps/desktop/src/renderer/react-query/projects/useOpenProject.tsxapps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/components/ProjectSettings/ProjectSettings.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx
✅ Files skipped from review due to trivial changes (1)
- apps/desktop/src/renderer/components/WorktreeChoiceDialog/WorktreeChoiceDialog.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/renderer/react-query/projects/useOpenProject.tsx
| if (input.trash) { | ||
| // Move to Trash (recoverable) instead of permanent delete | ||
| const { existsSync } = await import("node:fs"); | ||
| if (existsSync(worktree.path)) { | ||
| const { shell } = await import("electron"); | ||
| await shell.trashItem(worktree.path); | ||
| } | ||
| // Clean up stale git worktree references | ||
| const { getSimpleGitWithShellPath } = await import( | ||
| "../utils/git-client" | ||
| ); | ||
| const git = await getSimpleGitWithShellPath(project.mainRepoPath); | ||
| await git.raw(["worktree", "prune"]); |
There was a problem hiding this comment.
Redundant import and missing error handling in trash path.
existsSyncis already imported at line 1 - the dynamic import on line 258 is unnecessary.- Unlike the non-trash path (lines 270-277) which handles failures gracefully, the trash path has no error handling. If
shell.trashItemorgit worktree prunefails, the procedure silently continues and deletes the workspace record, potentially leaving inconsistent state.
Proposed fix
if (input.trash) {
// Move to Trash (recoverable) instead of permanent delete
- const { existsSync } = await import("node:fs");
if (existsSync(worktree.path)) {
- const { shell } = await import("electron");
- await shell.trashItem(worktree.path);
+ try {
+ const { shell } = await import("electron");
+ await shell.trashItem(worktree.path);
+ } catch (error) {
+ console.error(
+ `[workspace/delete] Failed to move worktree to trash:`,
+ error instanceof Error ? error.message : String(error),
+ );
+ clearWorkspaceDeletingStatus(input.id);
+ return {
+ success: false,
+ error: `Failed to move worktree to trash: ${error instanceof Error ? error.message : String(error)}`,
+ };
+ }
}
// Clean up stale git worktree references
- const { getSimpleGitWithShellPath } = await import(
- "../utils/git-client"
- );
- const git = await getSimpleGitWithShellPath(project.mainRepoPath);
- await git.raw(["worktree", "prune"]);
+ try {
+ const { getSimpleGitWithShellPath } = await import(
+ "../utils/git-client"
+ );
+ const git = await getSimpleGitWithShellPath(project.mainRepoPath);
+ await git.raw(["worktree", "prune"]);
+ } catch (error) {
+ // Non-blocking: prune failure shouldn't prevent workspace deletion
+ console.warn(
+ `[workspace/delete] git worktree prune failed (non-blocking):`,
+ error instanceof Error ? error.message : String(error),
+ );
+ }
} else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts` around
lines 256 - 268, Remove the redundant dynamic import of existsSync (use the
top-level existsSync already imported) and wrap the trash-path operations
(shell.trashItem and the subsequent getSimpleGitWithShellPath +
git.raw(["worktree","prune"])) in a try/catch; on error log the failure (using
the existing logger) and abort/throw a TRPC-friendly error so the workspace
record is not deleted, mirroring the non-trash path’s graceful failure handling.
| const handleDisableWorktrees = async (recycleWorktrees: boolean) => { | ||
| // Close/delete all worktree workspaces | ||
| for (const ws of worktreeWorkspaces) { | ||
| try { | ||
| if (recycleWorktrees) { | ||
| await deleteWorkspace.mutateAsync({ id: ws.id, trash: true }); | ||
| } else { | ||
| await closeWorkspaceMutation.mutateAsync({ id: ws.id }); | ||
| } | ||
| } catch (error) { | ||
| console.error( | ||
| `[ProjectSettings] Failed to close worktree workspace ${ws.id}:`, | ||
| error, | ||
| ); | ||
| } | ||
| } | ||
| // Now disable worktrees on the project | ||
| updateProject.mutate({ | ||
| id: projectId, | ||
| patch: { worktreeMode: "disabled" }, | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Use mutateAsync for the final project update and consider user feedback.
Two concerns with handleDisableWorktrees:
-
mutatevsmutateAsync: Line 264 usesmutatewhich doesn't await completion. If the project update fails after workspaces are already deleted/closed, the user won't know and the project remains in an inconsistent state (worktrees mode not disabled but workspaces gone). -
Silent failures: Individual workspace close/delete errors are only logged to console. Consider aggregating failures and showing a toast summary so users know if some workspaces couldn't be cleaned up.
Proposed fix for mutateAsync
const handleDisableWorktrees = async (recycleWorktrees: boolean) => {
+ const failures: string[] = [];
// Close/delete all worktree workspaces
for (const ws of worktreeWorkspaces) {
try {
if (recycleWorktrees) {
await deleteWorkspace.mutateAsync({ id: ws.id, trash: true });
} else {
await closeWorkspaceMutation.mutateAsync({ id: ws.id });
}
} catch (error) {
console.error(
`[ProjectSettings] Failed to close worktree workspace ${ws.id}:`,
error,
);
+ failures.push(ws.name);
}
}
+ if (failures.length > 0) {
+ toast.error(`Failed to close: ${failures.join(", ")}`);
+ }
// Now disable worktrees on the project
- updateProject.mutate({
+ try {
+ await updateProject.mutateAsync({
+ id: projectId,
+ patch: { worktreeMode: "disabled" },
+ });
+ } catch (error) {
+ toast.error("Failed to disable worktrees for project");
+ }
- updateProject.mutate({
- id: projectId,
- patch: { worktreeMode: "disabled" },
- });
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/project/`$projectId/components/ProjectSettings/ProjectSettings.tsx
around lines 247 - 268, The handleDisableWorktrees function currently uses
updateProject.mutate (non-awaiting) and only console.errors for individual
workspace failures; change updateProject.mutate to await
updateProject.mutateAsync({ id: projectId, patch: { worktreeMode: "disabled" }
}) so the final project update is performed atomically after workspace
operations, and capture/aggregate errors from deleteWorkspace.mutateAsync and
closeWorkspaceMutation.mutateAsync into a list; after all attempts, show a
user-facing toast (success/on partial failure/error) summarizing which workspace
IDs failed and only mark worktrees disabled on successful mutateAsync
completion, handling and surfacing any updateProject.mutateAsync error to the
user as well.
Branch-only projects now show the same hover UX as worktree workspaces: diff stats fade out and keyboard shortcut (⌘1-9) fades in on hover. Uses the same CSS grid overlay pattern as WorkspaceListItem.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectHeader.tsx`:
- Around line 354-356: Click currently becomes undefined when isBranchOnly is
true but onNavigateToWorkspace wasn't passed; update the ProjectHeader click
handler so it always has a fallback. Change the onClick to use
onNavigateToWorkspace if provided else fall back to onToggleCollapse (e.g.,
onClick={isBranchOnly ? (onNavigateToWorkspace ?? onToggleCollapse) :
onToggleCollapse}), and optionally log a console.warn when onNavigateToWorkspace
is missing to aid debugging.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ce52078e-bf83-4856-9b43-82dc745b1da0
📒 Files selected for processing (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectHeader.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectSection.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectSection.tsx
| onClick={ | ||
| isBranchOnly ? onNavigateToWorkspace : onToggleCollapse | ||
| } |
There was a problem hiding this comment.
Clicking does nothing if onNavigateToWorkspace is undefined in branch-only mode.
When isBranchOnly is true but onNavigateToWorkspace wasn't provided, the click handler becomes undefined and clicking silently does nothing. Consider adding a fallback or warning:
🛡️ Proposed defensive fix
onClick={
- isBranchOnly ? onNavigateToWorkspace : onToggleCollapse
+ isBranchOnly
+ ? (onNavigateToWorkspace ?? onToggleCollapse)
+ : onToggleCollapse
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| onClick={ | |
| isBranchOnly ? onNavigateToWorkspace : onToggleCollapse | |
| } | |
| onClick={ | |
| isBranchOnly | |
| ? (onNavigateToWorkspace ?? onToggleCollapse) | |
| : onToggleCollapse | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectHeader.tsx`
around lines 354 - 356, Click currently becomes undefined when isBranchOnly is
true but onNavigateToWorkspace wasn't passed; update the ProjectHeader click
handler so it always has a fallback. Change the onClick to use
onNavigateToWorkspace if provided else fall back to onToggleCollapse (e.g.,
onClick={isBranchOnly ? (onNavigateToWorkspace ?? onToggleCollapse) :
onToggleCollapse}), and optionally log a console.warn when onNavigateToWorkspace
is missing to aid debugging.
There was a problem hiding this comment.
5 issues found across 7 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx">
<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx:138">
P2: Workspace close behavior is inconsistent: only context-menu close supports recycle-to-trash, while X/middle-click/collapsed close still route through the legacy delete dialog without recycle.</violation>
</file>
<file name="apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts">
<violation number="1" location="apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts:261">
P1: Trash-mode deletion lacks failure cleanup; thrown errors can leave `deletingAt` stuck and block future deletes.</violation>
<violation number="2" location="apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts:268">
P2: Trash-delete path prunes worktrees without immediate expiry, which can leave just-removed worktrees registered in Git and desync app state from repository state.</violation>
</file>
<file name="apps/desktop/src/renderer/react-query/projects/useOpenProject.tsx">
<violation number="1" location="apps/desktop/src/renderer/react-query/projects/useOpenProject.tsx:95">
P1: Async errors in the worktree-choice callback can prevent `resolveChoice()` from running, leaving the outer awaited Promise pending and hanging the project-open workflow.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/components/ProjectSettings/ProjectSettings.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/components/ProjectSettings/ProjectSettings.tsx:264">
P1: Project worktree mode is set to disabled even when closing/deleting some worktree workspaces fails, allowing inconsistent state.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| const { existsSync } = await import("node:fs"); | ||
| if (existsSync(worktree.path)) { | ||
| const { shell } = await import("electron"); | ||
| await shell.trashItem(worktree.path); |
There was a problem hiding this comment.
P1: Trash-mode deletion lacks failure cleanup; thrown errors can leave deletingAt stuck and block future deletes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts, line 261:
<comment>Trash-mode deletion lacks failure cleanup; thrown errors can leave `deletingAt` stuck and block future deletes.</comment>
<file context>
@@ -252,13 +253,28 @@ export const createDeleteProcedures = () => {
+ const { existsSync } = await import("node:fs");
+ if (existsSync(worktree.path)) {
+ const { shell } = await import("electron");
+ await shell.trashItem(worktree.path);
+ }
+ // Clean up stale git worktree references
</file context>
When an agent is running in a branch-only project's workspace, the project thumbnail in the sidebar gets a colored ring: - Working (amber): pulsing ring while agent is processing - Permission (red): pulsing ring when agent needs user input - Review (green): static ring when agent finished but user hasn't seen it Uses the same tabs store status aggregation as WorkspaceListItem.
When a project is opened or created, automatically search for favicon/logo files in the repo (favicon.ico, logo.png, public/favicon, etc.) and set it as the project icon. Only runs if no icon is already set. Manually uploaded icons take priority. Fire-and-forget so it doesn't block project creation.
There was a problem hiding this comment.
5 issues found across 8 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/components/ProjectSettings/ProjectSettings.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/components/ProjectSettings/ProjectSettings.tsx:793">
P2: Disable-worktrees actions can be triggered concurrently because there is no in-flight guard/disabled state while async cleanup mutations are running.</violation>
</file>
<file name="apps/desktop/src/renderer/react-query/projects/useOpenProject.tsx">
<violation number="1" location="apps/desktop/src/renderer/react-query/projects/useOpenProject.tsx:95">
P1: Async errors in worktree-choice callback can prevent `resolveChoice()` from running, leaving `maybePromptWorktreeChoice` pending and hanging the open-project flow.</violation>
</file>
<file name="apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts">
<violation number="1" location="apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts:261">
P1: Trash-mode delete lacks failure cleanup, so thrown async errors can leave `deletingAt` set and block future deletion attempts.</violation>
</file>
<file name="apps/desktop/src/lib/trpc/routers/projects/projects.ts">
<violation number="1" location="apps/desktop/src/lib/trpc/routers/projects/projects.ts:122">
P2: Auto-discovery treats `iconUrl: null` as missing and re-runs after user removal, so cleared project icons are not persistent.</violation>
<violation number="2" location="apps/desktop/src/lib/trpc/routers/projects/projects.ts:155">
P2: `cloneRepo` bypasses `upsertProject`, so cloned projects skip the newly added favicon discovery/init behavior and diverge from other project creation flows.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| const { existsSync } = await import("node:fs"); | ||
| if (existsSync(worktree.path)) { | ||
| const { shell } = await import("electron"); | ||
| await shell.trashItem(worktree.path); |
There was a problem hiding this comment.
P1: Trash-mode delete lacks failure cleanup, so thrown async errors can leave deletingAt set and block future deletion attempts.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts, line 261:
<comment>Trash-mode delete lacks failure cleanup, so thrown async errors can leave `deletingAt` set and block future deletion attempts.</comment>
<file context>
@@ -252,13 +253,28 @@ export const createDeleteProcedures = () => {
+ const { existsSync } = await import("node:fs");
+ if (existsSync(worktree.path)) {
+ const { shell } = await import("electron");
+ await shell.trashItem(worktree.path);
+ }
+ // Clean up stale git worktree references
</file context>
| className="h-7 px-3 text-xs" | ||
| onClick={() => { | ||
| setShowDisableWorktreeDialog(false); | ||
| handleDisableWorktrees(false); |
There was a problem hiding this comment.
P2: Disable-worktrees actions can be triggered concurrently because there is no in-flight guard/disabled state while async cleanup mutations are running.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/components/ProjectSettings/ProjectSettings.tsx, line 793:
<comment>Disable-worktrees actions can be triggered concurrently because there is no in-flight guard/disabled state while async cleanup mutations are running.</comment>
<file context>
@@ -703,6 +748,67 @@ export function ProjectSettings({
+ className="h-7 px-3 text-xs"
+ onClick={() => {
+ setShowDisableWorktreeDialog(false);
+ handleDisableWorktrees(false);
+ }}
+ >
</file context>
| @@ -7,6 +7,7 @@ import { | |||
| projects, | |||
There was a problem hiding this comment.
P2: cloneRepo bypasses upsertProject, so cloned projects skip the newly added favicon discovery/init behavior and diverge from other project creation flows.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/lib/trpc/routers/projects/projects.ts, line 155:
<comment>`cloneRepo` bypasses `upsertProject`, so cloned projects skip the newly added favicon discovery/init behavior and diverge from other project creation flows.</comment>
<file context>
@@ -131,6 +152,24 @@ function upsertProject(mainRepoPath: string, defaultBranch: string): Project {
.returning()
.get();
+ // Discover favicon for new project (fire-and-forget)
+ discoverAndSaveProjectIcon({
+ projectId: project.id,
</file context>
| .run(); | ||
|
|
||
| // Discover favicon if no icon is set yet (fire-and-forget) | ||
| if (!existing.iconUrl) { |
There was a problem hiding this comment.
P2: Auto-discovery treats iconUrl: null as missing and re-runs after user removal, so cleared project icons are not persistent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/lib/trpc/routers/projects/projects.ts, line 122:
<comment>Auto-discovery treats `iconUrl: null` as missing and re-runs after user removal, so cleared project icons are not persistent.</comment>
<file context>
@@ -117,6 +117,27 @@ function upsertProject(mainRepoPath: string, defaultBranch: string): Project {
.run();
+
+ // Discover favicon if no icon is set yet (fire-and-forget)
+ if (!existing.iconUrl) {
+ discoverAndSaveProjectIcon({
+ projectId: existing.id,
</file context>
Add patterns for Next.js app directory (app/favicon.ico, app/icon.png), monorepo nested public directories (**/public/favicon.*), and increase glob depth to 4 levels. Fixes discovery for projects where the favicon is in a subdirectory like NEXTapp/app/ or peliguard-website/app/.
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/lib/trpc/routers/projects/utils/favicon-discovery.ts">
<violation number="1" location="apps/desktop/src/lib/trpc/routers/projects/utils/favicon-discovery.ts:41">
P2: Selecting `matches[0]` is now fragile because fast-glob does not guarantee pattern-priority ordering, so newly added recursive patterns can cause incorrect icon choice.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| "**/app/icon.png", | ||
| "**/app/icon.svg", | ||
| // Deeper public directories in monorepos/nested projects | ||
| "**/public/favicon.ico", |
There was a problem hiding this comment.
P2: Selecting matches[0] is now fragile because fast-glob does not guarantee pattern-priority ordering, so newly added recursive patterns can cause incorrect icon choice.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/lib/trpc/routers/projects/utils/favicon-discovery.ts, line 41:
<comment>Selecting `matches[0]` is now fragile because fast-glob does not guarantee pattern-priority ordering, so newly added recursive patterns can cause incorrect icon choice.</comment>
<file context>
@@ -28,6 +30,19 @@ const FAVICON_PATTERNS = [
+ "**/app/icon.png",
+ "**/app/icon.svg",
+ // Deeper public directories in monorepos/nested projects
+ "**/public/favicon.ico",
+ "**/public/favicon.png",
+ "**/public/favicon.svg",
</file context>
…rktree-mode # Conflicts: # apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectSection.tsx
- Checkout target branch before creating branch workspace when worktrees are disabled (prevents main repo staying on wrong branch) - Handle async errors in worktree choice dialog to prevent fire-and-forget rejections and ensure dialog closes properly - Wrap useOpenProject's onChoice in try/finally to guarantee resolveChoice() is always called, preventing indefinite pending state
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx">
<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx:405">
P2: Close behavior is inconsistently migrated: only one trigger uses `CloseWorkspaceDialog`, while other close paths still invoke the delete-dialog handler.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| handleDeleteClick(); | ||
| setShowCloseDialog(true); |
There was a problem hiding this comment.
P2: Close behavior is inconsistently migrated: only one trigger uses CloseWorkspaceDialog, while other close paths still invoke the delete-dialog handler.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx, line 405:
<comment>Close behavior is inconsistently migrated: only one trigger uses `CloseWorkspaceDialog`, while other close paths still invoke the delete-dialog handler.</comment>
<file context>
@@ -402,7 +402,7 @@ export function WorkspaceListItem({
onClick={(e) => {
e.stopPropagation();
- handleDeleteClick();
+ setShowCloseDialog(true);
}}
className="flex items-center justify-center text-muted-foreground hover:text-foreground"
</file context>
…rktree-mode # Conflicts: # apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts # apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx # apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectSection.tsx
Summary
Why / Context
Users working on projects that don't need worktree isolation (e.g. simple local folders) were forced through the full worktree workflow. This PR lets users choose at project-open time whether to enable worktrees, and adjusts all downstream UX accordingly.
How It Works
Worktree Choice on Project Open
When opening a new project folder, a dialog asks "Enable worktrees for [project]?" with two options:
worktreeMode: "disabled", single branch workspace, simplified sidebarSidebar for Non-Worktree Projects
Branch-only projects render as a single unified row: project thumbnail + name + branch name + diff stats. No collapse arrow, no "+" button, no nested workspace list. Clicking navigates directly.
Project Settings Reorganization
Close with Worktree Cleanup
Both project close and workspace close dialogs offer 3 options:
shell.trashItem)After trashing,
git worktree prunecleans up stale references.New Workspace Modal Cleanup
Manual QA Checklist
Worktree Choice Flow
Sidebar (Non-Worktree Projects)
Sidebar (Worktree Projects)
Project Settings
Close Project
Close Workspace
New Workspace Modal
Testing
bun run typecheck— passes (only pre-existing errors in useCreateWorkspace.ts)bun x @biomejs/biome check— cleanNew Files
WorktreeChoiceDialog/— dialog asking worktree yes/no on project openConnectedWorktreeChoiceDialog.tsx— connected to Zustand storeworktree-choice-dialog.ts— Zustand store for dialog stateCloseWorkspaceDialog/— workspace close confirmation with trash optionKnown Limitations
Summary by CodeRabbit
New Features
UX Improvements