feat: add file tree as a draggable pane#2536
Open
z3thon wants to merge 49 commits intosuperset-sh:mainfrom
Open
feat: add file tree as a draggable pane#2536z3thon wants to merge 49 commits intosuperset-sh:mainfrom
z3thon wants to merge 49 commits intosuperset-sh:mainfrom
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"
Contributor
|
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:
📝 WalkthroughWalkthroughIntroduces File Tree pane type with associated component and routing, adds worktree mode configuration system (always/optional/disabled) with settings UI, implements workspace close/trash workflows with dialogs, extends project settings for worktree management, and refactors right sidebar to support left-panel positioning with tab docking. Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as Project Open Dialog
participant Store as Project Store
participant TRPC as Backend TRPC
participant Dialog as Worktree Choice Dialog
participant DB as Database
User->>UI: Open new project
UI->>TRPC: openFromPath(path)
TRPC->>DB: Create project (no worktreeMode)
DB-->>TRPC: Project created
TRPC-->>UI: Project returned
UI->>Store: maybePromptWorktreeChoice()
alt Project missing worktreeMode
Store->>Dialog: open({ projectName, onChoice })
Dialog-->>User: Show enable/disable worktrees?
User->>Dialog: Select choice
Dialog->>TRPC: projects.update({ worktreeMode })
TRPC->>DB: Update project.worktreeMode
DB-->>TRPC: Saved
TRPC-->>Store: Success
end
Store->>TRPC: workspaces.getAllGrouped()
TRPC->>DB: Query workspaces/worktrees
DB-->>TRPC: Data
TRPC-->>Store: Workspaces loaded
Store-->>UI: Complete
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly Related Issues
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/desktop/src/renderer/stores/tabs/store.ts (1)
1276-1292: Extract the split-pane dispatch into one helper.The
paneType→ factory andpaneType→ analytics label mappings are now duplicated in both split methods. Adding"file-tree"already required four edits here; the next pane type can easily update one branch and miss another.♻️ Refactor sketch
+const buildSplitPane = ( + tabId: string, + paneType = "terminal", + options?: CreatePaneOptions, +) => { + switch (paneType) { + case "chat-mastra": + return { pane: createChatMastraPane(tabId), panelType: "chat" }; + case "webview": + return { pane: createBrowserPane(tabId), panelType: "browser" }; + case "file-tree": + return { pane: createFileTreePane(tabId), panelType: "file-tree" }; + default: + return { pane: createPane(tabId, "terminal", options), panelType: "terminal" }; + } +}; -const newPane = ... -const panelType = ... +const { pane: newPane, panelType } = buildSplitPane(tabId, paneType, options);Also applies to: 1349-1365
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/stores/tabs/store.ts` around lines 1276 - 1292, The pane-type-to-factory and pane-type-to-panelLabel logic is duplicated; create a single helper (e.g., getPaneAndPanelType or resolvePaneSpec) that accepts (paneType, tabId, options) and returns { pane: Pane, panelType: string } by centralizing the branching currently using createChatMastraPane, createBrowserPane, createFileTreePane, and createPane; replace the duplicated blocks in both split methods (the block that builds newPane and the block that computes panelType) to call this helper so future pane types are added in one place.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileTreePane/FileTreePane.tsx (1)
6-6: Prefer tsconfig alias import forFilesViewinstead of deep relative path.This keeps imports consistent and more resilient to file moves.
♻️ Suggested change
-import { FilesView } from "../../../../RightSidebar/FilesView"; +import { FilesView } from "renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView";As per coding guidelines
apps/desktop/**/*.{ts,tsx}: Use alias as defined in tsconfig.json when possible.🤖 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/WorkspaceView/ContentView/TabsContent/TabView/FileTreePane/FileTreePane.tsx` at line 6, Update the import in FileTreePane.tsx to use the project's tsconfig path alias instead of the deep relative path; locate the import statement that references FilesView and replace the "../../../../RightSidebar/FilesView" style import with the path-alias form defined in tsconfig (keeping the same exported symbol name FilesView) so future file moves and refactors remain resilient.
🤖 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/WorkspaceView/ContentView/components/PaneContextMenuItems/PaneContextMenuItems.tsx`:
- Around line 85-90: ChatMastraPane is not forwarding the onSplitWithFileTree
callback so the "Split with File Tree" menu item never appears for Chat Mastra
panes; update ChatMastraPane (the component and its props) to accept
onSplitWithFileTree and pass it through into the actions object/prop that is
given to PaneContextMenuItems (the same prop name actions.onSplitWithFileTree
used by TabPane and FileTreePane). Ensure the ChatMastraPane props/interface
includes onSplitWithFileTree, that the handler is forwarded from the parent
where available, and that the actions object passed into PaneContextMenuItems
contains the onSplitWithFileTree reference so the menu item renders and invokes
the callback.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileTreePane/FileTreePane.tsx`:
- Line 6: Update the import in FileTreePane.tsx to use the project's tsconfig
path alias instead of the deep relative path; locate the import statement that
references FilesView and replace the "../../../../RightSidebar/FilesView" style
import with the path-alias form defined in tsconfig (keeping the same exported
symbol name FilesView) so future file moves and refactors remain resilient.
In `@apps/desktop/src/renderer/stores/tabs/store.ts`:
- Around line 1276-1292: The pane-type-to-factory and pane-type-to-panelLabel
logic is duplicated; create a single helper (e.g., getPaneAndPanelType or
resolvePaneSpec) that accepts (paneType, tabId, options) and returns { pane:
Pane, panelType: string } by centralizing the branching currently using
createChatMastraPane, createBrowserPane, createFileTreePane, and createPane;
replace the duplicated blocks in both split methods (the block that builds
newPane and the block that computes panelType) to call this helper so future
pane types are added in one place.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 65b69b00-09a1-41da-83f0-1a389027d8dd
📒 Files selected for processing (10)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabContentContextMenu.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileTreePane/FileTreePane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileTreePane/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/TabPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/components/PaneContextMenuItems/PaneContextMenuItems.tsxapps/desktop/src/renderer/stores/tabs/store.tsapps/desktop/src/renderer/stores/tabs/types.tsapps/desktop/src/renderer/stores/tabs/utils.tsapps/desktop/src/shared/tabs-types.ts
Comment on lines
+85
to
+90
| {actions.onSplitWithFileTree && ( | ||
| <ContextMenuItem onSelect={actions.onSplitWithFileTree}> | ||
| <LuFolderTree className="size-4" /> | ||
| Split with File Tree | ||
| </ContextMenuItem> | ||
| )} |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "TabContentContextMenu JSX call sites:"
rg -n -C4 --glob '*.tsx' '<TabContentContextMenu' apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent
echo
echo "Call sites already wiring onSplitWithFileTree:"
rg -n -C2 --glob '*.tsx' 'onSplitWithFileTree\s*=' apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContentRepository: superset-sh/superset
Length of output: 5965
Wire onSplitWithFileTree from ChatMastraPane.
The "Split with File Tree" menu item only renders when onSplitWithFileTree is supplied. While TabPane.tsx and FileTreePane.tsx wire this callback, ChatMastraPane.tsx does not, leaving the feature unavailable when viewing Chat Mastra panes.
- ✓
TabPane.tsx(line 131):onSplitWithFileTreepassed - ✓
FileTreePane.tsx(line 99):onSplitWithFileTreepassed - ✗
ChatMastraPane.tsx(line 192):onSplitWithFileTreemissing
🤖 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/WorkspaceView/ContentView/components/PaneContextMenuItems/PaneContextMenuItems.tsx`
around lines 85 - 90, ChatMastraPane is not forwarding the onSplitWithFileTree
callback so the "Split with File Tree" menu item never appears for Chat Mastra
panes; update ChatMastraPane (the component and its props) to accept
onSplitWithFileTree and pass it through into the actions object/prop that is
given to PaneContextMenuItems (the same prop name actions.onSplitWithFileTree
used by TabPane and FileTreePane). Ensure the ChatMastraPane props/interface
includes onSplitWithFileTree, that the handler is forwarded from the parent
where available, and that the actions object passed into PaneContextMenuItems
contains the onSplitWithFileTree reference so the menu item renders and invokes
the callback.
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
…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"
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.
z3thon
force-pushed
the
feat/file-tree-pane
branch
2 times, most recently
from
64bfbe6 to
319c357
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/ProjectsSettings.tsx (1)
14-27:⚠️ Potential issue | 🟠 MajorRemove unused
ProjectsSettingscomponent.The
ProjectsSettingscomponent atapps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/ProjectsSettings.tsxis not imported or used anywhere in the codebase. The entire component, including thesearchQueryprop, should be deleted.🤖 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/components/SettingsSidebar/ProjectsSettings.tsx` around lines 14 - 27, Delete the unused ProjectsSettings component and its props: remove the entire ProjectsSettings function (including its props signature ProjectsSettingsProps and references to searchQuery, matchCounts, hasProjectMatches, useMatchRoute, and hasCloudAccess) from the codebase; also remove the file ProjectsSettings.tsx and any leftover type or import definitions that only served this component so there are no unused-export or import warnings. Ensure no other modules import ProjectsSettings before deleting (search for the symbol ProjectsSettings) and run the type checker to confirm no remaining references.
🧹 Nitpick comments (7)
apps/desktop/src/lib/trpc/routers/workspaces/procedures/query.ts (1)
155-155: KeepworktreeModestrongly typed instead of widening tostring.Using
string | nullon Line 155 drops the enum contract and weakens downstream type safety. PreferWorktreeMode | nullhere.♻️ Suggested type-safe diff
import { projects, + type WorktreeMode, workspaceSections, workspaces, worktrees, } from "@superset/local-db"; @@ - worktreeMode: string | null; + worktreeMode: WorktreeMode | null; @@ - worktreeMode: project.worktreeMode ?? null, + worktreeMode: project.worktreeMode ?? null,Also applies to: 188-188
🤖 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` at line 155, Replace the widened type annotation "worktreeMode: string | null" with the enum type "WorktreeMode | null" wherever it appears in this file (e.g., the query result/interface declaration and the secondary occurrence on the same file), and import or reference the WorktreeMode enum/type so the compiler sees it; ensure any functions consuming worktreeMode (e.g., the query resolver or mapping logic) still accept WorktreeMode | null rather than a plain string to preserve downstream type safety.apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx (1)
124-142: LGTM with a minor suggestion.The close workspace flow is well-implemented with proper mutation handling and cache invalidation. The routing logic in
handleCloseConfirmcorrectly differentiates between simple close and trash operations.Consider making the error message more specific for the delete mutation since it performs a different operation:
♻️ Suggested error message improvement
const deleteWorkspace = electronTrpc.workspaces.delete.useMutation({ onSuccess: () => utils.workspaces.getAllGrouped.invalidate(), onError: (error) => - toast.error(`Failed to close workspace: ${error.message}`), + toast.error(`Failed to recycle workspace: ${error.message}`), });🤖 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, Update the deleteWorkspace mutation's onError handler to log a specific message for delete failures (referencing deleteWorkspace) instead of reusing the close message; modify the onError in deleteWorkspace to call toast.error with something like "Failed to delete workspace: <error.message>" and keep the existing cache invalidation logic, and ensure handleCloseConfirm continues to call deleteWorkspace.mutate({ id, trash: true }) for the trash flow.apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx (1)
915-919: Minor: Unnecessary wrapper div.The wrapper
<div className="flex items-center gap-2">withgap-2is applied but there's only one child element (the<span>), making thegap-2and flex layout ineffective.♻️ Suggested simplification
- <div className="flex items-center gap-2"> - <span className="text-[11px] text-muted-foreground/50"> - {modKey}+↵ to create - </span> - </div> + <span className="text-[11px] text-muted-foreground/50"> + {modKey}+↵ to create + </span>🤖 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 915 - 919, The wrapper div in PromptGroup around the shortcut hint is unnecessary because it only contains a single <span>; remove the <div className="flex items-center gap-2"> and apply any needed classes directly to the <span> (keep "text-[11px] text-muted-foreground/50" on the span or add "flex items-center" to the parent container if layout requires it) so the extraneous gap/flex markup is eliminated and the render remains identical; update the JSX in the PromptGroup component accordingly.apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts (1)
258-258: Redundant import ofexistsSync.
existsSyncis already imported at line 1. The dynamic import here is unnecessary.♻️ Remove redundant import
if (input.trash) { // Move to Trash (recoverable) instead of permanent delete - const { existsSync } = await import("node:fs"); if (existsSync(worktree.path)) {🤖 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` at line 258, The dynamic import line "const { existsSync } = await import('node:fs')" inside the delete procedure is redundant because existsSync is already imported at the module top; remove that dynamic import and reference the existing existsSync symbol directly (in the delete workspace procedure where the dynamic import appears) so the function uses the top-level imported existsSync and no duplicate import remains.apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectSection.tsx (1)
358-358: Redundant!isBranchOnlycheck.This condition is already guaranteed by the outer
{!isBranchOnly && ...}block at line 347.♻️ Simplify redundant condition
- {!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 358, The inner JSX conditional repeats the outer {!isBranchOnly && ...} guard, so remove the redundant "!isBranchOnly" from the inner expression that checks topLevelChildren.length === 0 in ProjectSection (referencing the topLevelChildren variable and isBranchOnly flag) so the inner render becomes just {topLevelChildren.length === 0 && (...)}; this keeps the outer guard intact and eliminates the duplicate check.apps/desktop/src/lib/trpc/routers/projects/projects.ts (1)
1407-1434: Redundant import and potentially duplicative worktree path collection.
Line 1409 re-imports
existsSync, but it's already imported at line 1.The nested loop (lines 1423-1434) appears redundant. The first query (lines 1413-1417) already fetches all worktrees for the project via
eq(worktrees.projectId, input.id). When iterating workspaces withworktreeId, those worktrees are already included inprojectWorktreessince they belong to the same project.Suggested simplification
// Optionally move worktree directories to Trash if (input.deleteWorktrees) { - const { existsSync } = await import("node:fs"); const { shell } = await import("electron"); // Collect worktree paths from both the worktrees table and workspace records const projectWorktrees = localDb .select() .from(worktrees) .where(eq(worktrees.projectId, input.id)) .all(); + // All worktrees for this project are already fetched above const worktreePaths = new Set<string>( projectWorktrees.map((wt) => wt.path), ); - for (const ws of projectWorkspaces) { - if (ws.type === "worktree" && ws.worktreeId) { - const wt = localDb - .select() - .from(worktrees) - .where(eq(worktrees.id, ws.worktreeId)) - .get(); - if (wt?.path) { - worktreePaths.add(wt.path); - } - } - } - for (const wtPath of worktreePaths) {apps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/components/ProjectSettings/ProjectSettings.tsx (1)
822-843: Consider disabling buttons during the async operation.When clicking "Disable" or "Disable & Recycle", the dialog closes immediately while
handleDisableWorktreesruns in the background. If the operation takes time (many workspaces), users might interact with the UI before it completes.Consider keeping the dialog open with a loading state until the operation finishes, or at minimum disabling the buttons.
🤖 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 822 - 843, The dialog currently closes immediately and allows UI interaction while handleDisableWorktrees runs; add a local boolean state (e.g., isDisablingWorktrees) and update the onClick handlers for the "Disable" and "Disable & Recycle" buttons to set isDisablingWorktrees = true, call an async wrapper that awaits handleDisableWorktrees(...), then set isDisablingWorktrees = false (and only close the dialog after the awaited call if you want it to close). Use isDisablingWorktrees to set disabled={isDisablingWorktrees} on both Button components (and optional loading indicator) and avoid calling setShowDisableWorktreeDialog(false) before the await so the dialog remains open/disabled until the operation completes.
🤖 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-278: The trash branch lacks error handling: wrap the block
that calls shell.trashItem(worktree.path) and git.raw(["worktree", "prune"])
(obtained via getSimpleGitWithShellPath(project.mainRepoPath)) in a try/catch
and on any error call clearWorkspaceDeletingStatus(input.id) and return a
failure result consistent with removeWorktreeFromDisk's shape (so the caller can
handle the failure), ensuring worktree.path and project.mainRepoPath remain
referenced and no exception escapes the procedure.
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/components/NewWorkspaceModalContent/NewWorkspaceModalContent.tsx`:
- Around line 87-90: Create a single derived list (e.g., selectableProjects)
from recentProjects by filtering out items without an id and those with
worktreeMode === "disabled", and then use this selectableProjects everywhere the
component builds selection options and initializes default/preselected values
(replace usages of recentProjects in the preselection/default-selection logic
with selectableProjects); ensure the recentProjects prop passed to the child
options and any functions like the default selection initializer or selection
state (inside NewWorkspaceModalContent) reference selectableProjects so a
disabled project cannot be preselected when it is not shown in the options.
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/project/`$projectId/components/ProjectSettings/ProjectSettings.tsx:
- Around line 246-267: The final project update in handleDisableWorktrees is not
awaited, causing a race where the dialog/UI may close before worktreeMode is
persisted; change the final call to await the async mutation (e.g., await
updateProject.mutateAsync({ id: projectId, patch: { worktreeMode: "disabled" }
})) so the function only returns after the project update completes—this ensures
the sequence following worktreeWorkspaces handling (and the earlier
deleteWorkspace.mutateAsync / closeWorkspaceMutation.mutateAsync calls) finishes
before the dialog closes.
In `@apps/desktop/src/renderer/stores/worktree-choice-dialog.ts`:
- Around line 22-27: The close() handler in worktree-choice-dialog.ts currently
clears onChoice without invoking it, which can leave the promise in
useOpenProject.tsx unresolved; modify close() to first read the current onChoice
(the same callback passed into open()), and if it exists call it (e.g., invoke
onChoice(false) to signal cancellation) before calling set({ isOpen: false,
projectName: "", onChoice: null }); ensure you reference the existing open and
close methods in the store (open: ({ projectName, onChoice }) and close: () => {
... }) so the caller in useOpenProject.tsx receives a resolution when the dialog
is dismissed.
---
Outside diff comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/ProjectsSettings.tsx`:
- Around line 14-27: Delete the unused ProjectsSettings component and its props:
remove the entire ProjectsSettings function (including its props signature
ProjectsSettingsProps and references to searchQuery, matchCounts,
hasProjectMatches, useMatchRoute, and hasCloudAccess) from the codebase; also
remove the file ProjectsSettings.tsx and any leftover type or import definitions
that only served this component so there are no unused-export or import
warnings. Ensure no other modules import ProjectsSettings before deleting
(search for the symbol ProjectsSettings) and run the type checker to confirm no
remaining references.
---
Nitpick comments:
In `@apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts`:
- Line 258: The dynamic import line "const { existsSync } = await
import('node:fs')" inside the delete procedure is redundant because existsSync
is already imported at the module top; remove that dynamic import and reference
the existing existsSync symbol directly (in the delete workspace procedure where
the dynamic import appears) so the function uses the top-level imported
existsSync and no duplicate import remains.
In `@apps/desktop/src/lib/trpc/routers/workspaces/procedures/query.ts`:
- Line 155: Replace the widened type annotation "worktreeMode: string | null"
with the enum type "WorktreeMode | null" wherever it appears in this file (e.g.,
the query result/interface declaration and the secondary occurrence on the same
file), and import or reference the WorktreeMode enum/type so the compiler sees
it; ensure any functions consuming worktreeMode (e.g., the query resolver or
mapping logic) still accept WorktreeMode | null rather than a plain string to
preserve downstream type safety.
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx`:
- Around line 915-919: The wrapper div in PromptGroup around the shortcut hint
is unnecessary because it only contains a single <span>; remove the <div
className="flex items-center gap-2"> and apply any needed classes directly to
the <span> (keep "text-[11px] text-muted-foreground/50" on the span or add "flex
items-center" to the parent container if layout requires it) so the extraneous
gap/flex markup is eliminated and the render remains identical; update the JSX
in the PromptGroup component accordingly.
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/project/`$projectId/components/ProjectSettings/ProjectSettings.tsx:
- Around line 822-843: The dialog currently closes immediately and allows UI
interaction while handleDisableWorktrees runs; add a local boolean state (e.g.,
isDisablingWorktrees) and update the onClick handlers for the "Disable" and
"Disable & Recycle" buttons to set isDisablingWorktrees = true, call an async
wrapper that awaits handleDisableWorktrees(...), then set isDisablingWorktrees =
false (and only close the dialog after the awaited call if you want it to
close). Use isDisablingWorktrees to set disabled={isDisablingWorktrees} on both
Button components (and optional loading indicator) and avoid calling
setShowDisableWorktreeDialog(false) before the await so the dialog remains
open/disabled until the operation completes.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectSection.tsx`:
- Line 358: The inner JSX conditional repeats the outer {!isBranchOnly && ...}
guard, so remove the redundant "!isBranchOnly" from the inner expression that
checks topLevelChildren.length === 0 in ProjectSection (referencing the
topLevelChildren variable and isBranchOnly flag) so the inner render becomes
just {topLevelChildren.length === 0 && (...)}; this keeps the outer guard intact
and eliminates the duplicate check.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx`:
- Around line 124-142: Update the deleteWorkspace mutation's onError handler to
log a specific message for delete failures (referencing deleteWorkspace) instead
of reusing the close message; modify the onError in deleteWorkspace to call
toast.error with something like "Failed to delete workspace: <error.message>"
and keep the existing cache invalidation logic, and ensure handleCloseConfirm
continues to call deleteWorkspace.mutate({ id, trash: true }) for the trash
flow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 19e3e330-1cda-4c94-b685-dad5d16eb799
📒 Files selected for processing (42)
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/lib/trpc/routers/workspaces/procedures/delete.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/components/SettingsSidebar/GeneralSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/ProjectsSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/SettingsSidebar.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/projects/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/utils/settings-search/settings-search.tsapps/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/screens/main/components/WorkspaceView/ContentView/TabsContent/TabContentContextMenu.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileTreePane/FileTreePane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileTreePane/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/TabPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/components/PaneContextMenuItems/PaneContextMenuItems.tsxapps/desktop/src/renderer/stores/tabs/store.tsapps/desktop/src/renderer/stores/tabs/types.tsapps/desktop/src/renderer/stores/tabs/utils.tsapps/desktop/src/renderer/stores/worktree-choice-dialog.tsapps/desktop/src/shared/tabs-types.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
💤 Files with no reviewable changes (1)
- apps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/SettingsSidebar.tsx
✅ Files skipped from review due to trivial changes (9)
- apps/desktop/src/renderer/components/WorktreeChoiceDialog/index.ts
- packages/local-db/drizzle/meta/_journal.json
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/TabPane.tsx
- apps/desktop/src/shared/tabs-types.ts
- apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/components/CloseWorkspaceDialog/index.ts
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/index.tsx
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileTreePane/index.ts
- packages/local-db/drizzle/0037_add_worktree_mode.sql
- apps/desktop/src/renderer/stores/tabs/types.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/desktop/src/renderer/stores/tabs/utils.ts
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/components/PaneContextMenuItems/PaneContextMenuItems.tsx
- apps/desktop/src/renderer/stores/tabs/store.ts
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabContentContextMenu.tsx
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileTreePane/FileTreePane.tsx
Comment on lines
+256
to
278
| 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"]); | ||
| } else { | ||
| const removeResult = await removeWorktreeFromDisk({ | ||
| mainRepoPath: project.mainRepoPath, | ||
| worktreePath: worktree.path, | ||
| }); | ||
| if (!removeResult.success) { | ||
| clearWorkspaceDeletingStatus(input.id); | ||
| return removeResult; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Missing error handling for trash operation may leave workspace in "deleting" state.
The trash path (lines 256-268) doesn't handle errors from shell.trashItem or git.raw(["worktree", "prune"]). If either fails, the exception propagates but clearWorkspaceDeletingStatus(input.id) is never called, leaving the workspace permanently marked as deleting.
Compare to the non-trash path (lines 270-277) which properly calls clearWorkspaceDeletingStatus on failure before returning.
🔧 Proposed fix to add error handling
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 { 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"]);
+ } catch (error) {
+ clearWorkspaceDeletingStatus(input.id);
+ return {
+ success: false,
+ error: `Failed to trash worktree: ${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"]);
} 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 - 278, The trash branch lacks error handling: wrap the block that
calls shell.trashItem(worktree.path) and git.raw(["worktree", "prune"])
(obtained via getSimpleGitWithShellPath(project.mainRepoPath)) in a try/catch
and on any error call clearWorkspaceDeletingStatus(input.id) and return a
failure result consistent with removeWorktreeFromDisk's shape (so the caller can
handle the failure), ensuring worktree.path and project.mainRepoPath remain
referenced and no exception escapes the procedure.
Comment on lines
+87
to
+90
| recentProjects={recentProjects.filter( | ||
| (project) => | ||
| Boolean(project.id) && project.worktreeMode !== "disabled", | ||
| )} |
Contributor
There was a problem hiding this comment.
Selection logic is now inconsistent with the filtered project list.
Line 87-Line 90 filters disabled projects only at render time, but preselection/default selection still use unfiltered recentProjects (Line 49-Line 68). That can keep a disabled project selected while it is missing from the UI options.
✅ Suggested fix: derive one `selectableProjects` list and use it consistently
+ const selectableProjects = recentProjects.filter(
+ (project) =>
+ Boolean(project.id) && project.worktreeMode !== "disabled",
+ );
useEffect(() => {
if (!isOpen) return;
if (
preSelectedProjectId &&
preSelectedProjectId !== appliedPreSelectionRef.current
) {
if (!areRecentProjectsFetched) return;
- const hasPreSelectedProject = recentProjects.some(
+ const hasPreSelectedProject = selectableProjects.some(
(project) => project.id === preSelectedProjectId,
);
if (hasPreSelectedProject) {
appliedPreSelectionRef.current = preSelectedProjectId;
if (preSelectedProjectId !== draft.selectedProjectId) {
updateDraft({ selectedProjectId: preSelectedProjectId });
}
return;
}
}
if (!areRecentProjectsFetched) return;
- const hasSelectedProject = recentProjects.some(
+ const hasSelectedProject = selectableProjects.some(
(project) => project.id === draft.selectedProjectId,
);
if (!hasSelectedProject) {
- updateDraft({ selectedProjectId: recentProjects[0]?.id ?? null });
+ updateDraft({ selectedProjectId: selectableProjects[0]?.id ?? null });
}
}, [
draft.selectedProjectId,
areRecentProjectsFetched,
isOpen,
preSelectedProjectId,
recentProjects,
+ selectableProjects,
updateDraft,
]);
- const selectedProject = recentProjects.find(
+ const selectedProject = selectableProjects.find(
(project) => project.id === draft.selectedProjectId,
);
return (
<div className="flex-1 overflow-y-auto">
<PromptGroup
projectId={draft.selectedProjectId}
selectedProject={selectedProject}
- recentProjects={recentProjects.filter(
- (project) =>
- Boolean(project.id) && project.worktreeMode !== "disabled",
- )}
+ recentProjects={selectableProjects}📝 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.
Suggested change
| recentProjects={recentProjects.filter( | |
| (project) => | |
| Boolean(project.id) && project.worktreeMode !== "disabled", | |
| )} | |
| const selectableProjects = recentProjects.filter( | |
| (project) => | |
| Boolean(project.id) && project.worktreeMode !== "disabled", | |
| ); | |
| useEffect(() => { | |
| if (!isOpen) return; | |
| if ( | |
| preSelectedProjectId && | |
| preSelectedProjectId !== appliedPreSelectionRef.current | |
| ) { | |
| if (!areRecentProjectsFetched) return; | |
| const hasPreSelectedProject = selectableProjects.some( | |
| (project) => project.id === preSelectedProjectId, | |
| ); | |
| if (hasPreSelectedProject) { | |
| appliedPreSelectionRef.current = preSelectedProjectId; | |
| if (preSelectedProjectId !== draft.selectedProjectId) { | |
| updateDraft({ selectedProjectId: preSelectedProjectId }); | |
| } | |
| return; | |
| } | |
| } | |
| if (!areRecentProjectsFetched) return; | |
| const hasSelectedProject = selectableProjects.some( | |
| (project) => project.id === draft.selectedProjectId, | |
| ); | |
| if (!hasSelectedProject) { | |
| updateDraft({ selectedProjectId: selectableProjects[0]?.id ?? null }); | |
| } | |
| }, [ | |
| draft.selectedProjectId, | |
| areRecentProjectsFetched, | |
| isOpen, | |
| preSelectedProjectId, | |
| recentProjects, | |
| selectableProjects, | |
| updateDraft, | |
| ]); | |
| const selectedProject = selectableProjects.find( | |
| (project) => project.id === draft.selectedProjectId, | |
| ); | |
| return ( | |
| <div className="flex-1 overflow-y-auto"> | |
| <PromptGroup | |
| projectId={draft.selectedProjectId} | |
| selectedProject={selectedProject} | |
| recentProjects={selectableProjects} | |
| /> | |
| </div> | |
| ); |
🤖 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/NewWorkspaceModalContent/NewWorkspaceModalContent.tsx`
around lines 87 - 90, Create a single derived list (e.g., selectableProjects)
from recentProjects by filtering out items without an id and those with
worktreeMode === "disabled", and then use this selectableProjects everywhere the
component builds selection options and initializes default/preselected values
(replace usages of recentProjects in the preselection/default-selection logic
with selectableProjects); ensure the recentProjects prop passed to the child
options and any functions like the default selection initializer or selection
state (inside NewWorkspaceModalContent) reference selectableProjects so a
disabled project cannot be preselected when it is not shown in the options.
Comment on lines
+246
to
+267
| 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" }, | ||
| }); | ||
| }; |
Contributor
There was a problem hiding this comment.
Missing await on final mutation may cause race condition.
The handleDisableWorktrees function is async and awaits each workspace deletion, but the final updateProject.mutate() call on line 263 is not awaited. Since the dialog closes immediately after calling this function (lines 828-829, 838-839), the worktree mode update might not complete before the UI state changes.
Suggested fix
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({
+ await updateProject.mutateAsync({
id: projectId,
patch: { worktreeMode: "disabled" },
});
};📝 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.
Suggested change
| 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" }, | |
| }); | |
| }; | |
| 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 | |
| await updateProject.mutateAsync({ | |
| 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 246 - 267, The final project update in handleDisableWorktrees is
not awaited, causing a race where the dialog/UI may close before worktreeMode is
persisted; change the final call to await the async mutation (e.g., await
updateProject.mutateAsync({ id: projectId, patch: { worktreeMode: "disabled" }
})) so the function only returns after the project update completes—this ensures
the sequence following worktreeWorkspaces handling (and the earlier
deleteWorkspace.mutateAsync / closeWorkspaceMutation.mutateAsync calls) finishes
before the dialog closes.
Comment on lines
+22
to
+27
| open: ({ projectName, onChoice }) => { | ||
| set({ isOpen: true, projectName, onChoice }); | ||
| }, | ||
|
|
||
| close: () => { | ||
| set({ isOpen: false, projectName: "", onChoice: null }); |
Contributor
There was a problem hiding this comment.
Dialog dismissal can leave the open-project flow hanging.
On Line 26-Line 27, close() clears onChoice without invoking it. The caller in apps/desktop/src/renderer/react-query/projects/useOpenProject.tsx waits on a Promise that resolves only inside onChoice, so dismissing via onOpenChange can deadlock that flow.
💡 Suggested direction
interface WorktreeChoiceDialogState {
isOpen: boolean;
projectName: string;
- onChoice: ((enableWorktrees: boolean) => void) | null;
+ onChoice: ((enableWorktrees: boolean | null) => void) | null;
open: (params: {
projectName: string;
- onChoice: (enableWorktrees: boolean) => void;
+ onChoice: (enableWorktrees: boolean | null) => void;
}) => void;
close: () => void;
+ dismiss: () => void;
}
export const useWorktreeChoiceDialogStore = create<WorktreeChoiceDialogState>()(
devtools(
- (set) => ({
+ (set, get) => ({
isOpen: false,
projectName: "",
onChoice: null,
open: ({ projectName, onChoice }) => {
set({ isOpen: true, projectName, onChoice });
},
close: () => {
set({ isOpen: false, projectName: "", onChoice: null });
},
+ dismiss: () => {
+ const cb = get().onChoice;
+ set({ isOpen: false, projectName: "", onChoice: null });
+ cb?.(null);
+ },
}),
{ name: "WorktreeChoiceDialogStore" },
),
);📝 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.
Suggested change
| open: ({ projectName, onChoice }) => { | |
| set({ isOpen: true, projectName, onChoice }); | |
| }, | |
| close: () => { | |
| set({ isOpen: false, projectName: "", onChoice: null }); | |
| interface WorktreeChoiceDialogState { | |
| isOpen: boolean; | |
| projectName: string; | |
| onChoice: ((enableWorktrees: boolean | null) => void) | null; | |
| open: (params: { | |
| projectName: string; | |
| onChoice: (enableWorktrees: boolean | null) => void; | |
| }) => void; | |
| close: () => void; | |
| dismiss: () => void; | |
| } | |
| export const useWorktreeChoiceDialogStore = create<WorktreeChoiceDialogState>()( | |
| devtools( | |
| (set, get) => ({ | |
| isOpen: false, | |
| projectName: "", | |
| onChoice: null, | |
| open: ({ projectName, onChoice }) => { | |
| set({ isOpen: true, projectName, onChoice }); | |
| }, | |
| close: () => { | |
| set({ isOpen: false, projectName: "", onChoice: null }); | |
| }, | |
| dismiss: () => { | |
| const cb = get().onChoice; | |
| set({ isOpen: false, projectName: "", onChoice: null }); | |
| cb?.(null); | |
| }, | |
| }), | |
| { name: "WorktreeChoiceDialogStore" }, | |
| ), | |
| ); |
🤖 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 22 -
27, The close() handler in worktree-choice-dialog.ts currently clears onChoice
without invoking it, which can leave the promise in useOpenProject.tsx
unresolved; modify close() to first read the current onChoice (the same callback
passed into open()), and if it exists call it (e.g., invoke onChoice(false) to
signal cancellation) before calling set({ isOpen: false, projectName: "",
onChoice: null }); ensure you reference the existing open and close methods in
the store (open: ({ projectName, onChoice }) and close: () => { ... }) so the
caller in useOpenProject.tsx receives a resolution when the dialog is dismissed.
Contributor
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/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsx`:
- Around line 308-327: The expand toggle is an icon-only Button (rendered when
onExpandToggle is provided) and lacks accessible labeling/state; update the
Button in ChangesHeader so it includes an explicit aria-label (use the same text
as TooltipContent: isExpanded ? "Collapse" : "Expand") and set
aria-expanded={isExpanded} so screen readers get the current state; keep the
existing Tooltip/TooltipContent but ensure the Button element (the one with
variant="ghost" size="icon" onClick={onExpandToggle} and the icon children
LuShrink/LuExpand) receives these attributes.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/index.tsx`:
- Around line 109-114: The current per-side visibility flags (showChangesTab and
showFilesTab) only check tabPositions and thus a side can end up with no active
tab when tabs are split; change the logic so each side computes its
visibleTabsOnSide (count of RightSidebarTab.Changes visible here considering
worktreePath and RightSidebarTab.Files) and treat a tab as active if it is
assigned to this side OR (visibleTabsOnSide === 1 and that tab is the only
visible one on this side). Update the entries that set showChangesTab and
showFilesTab (and any similar checks later in the file around the referenced
ranges) to use this rule so a single split-out tab becomes active on its own
side. Ensure you reference RightSidebarTab, tabPositions, rightSidebarTab, and
worktreePath in the new logic.
- Around line 283-287: The tooltip currently shows the TOGGLE_SIDEBAR hotkey for
both sides but on the left the button only re-docks/ closes that pane; update
the HotkeyTooltipContent usage so when side === "left" it does not advertise the
TOGGLE_SIDEBAR hotkey: change the label to "Close left panel" and remove or set
hotkeyId to undefined/null for the left branch, and keep
hotkeyId="TOGGLE_SIDEBAR" only for the non-left (sidebar) branch; modify the
conditional around HotkeyTooltipContent to pick the correct label and hotkeyId
based on the side variable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f9faa44a-1f20-41e9-9248-67ee1268046a
📒 Files selected for processing (6)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileTreePane/FileTreePane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/ChangesView.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceLayout/WorkspaceLayout.tsxapps/desktop/src/renderer/stores/sidebar-state.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileTreePane/FileTreePane.tsx
Comment on lines
+308
to
+327
| {onExpandToggle && ( | ||
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <Button | ||
| variant="ghost" | ||
| size="icon" | ||
| onClick={onExpandToggle} | ||
| className="size-6 p-0" | ||
| > | ||
| {isExpanded ? ( | ||
| <LuShrink className="size-3.5" /> | ||
| ) : ( | ||
| <LuExpand className="size-3.5" /> | ||
| )} | ||
| </Button> | ||
| </TooltipTrigger> | ||
| <TooltipContent side="top" showArrow={false}> | ||
| {isExpanded ? "Collapse" : "Expand"} | ||
| </TooltipContent> | ||
| </Tooltip> |
Contributor
There was a problem hiding this comment.
Add an accessible name/state to the new expand toggle.
This is an icon-only button, so the control currently ships without a reliable announced label. Please set an explicit aria-label and aria-expanded from isExpanded.
Suggested fix
<TooltipTrigger asChild>
<Button
variant="ghost"
size="icon"
onClick={onExpandToggle}
className="size-6 p-0"
+ aria-label={
+ isExpanded ? "Collapse changes view" : "Expand changes view"
+ }
+ aria-expanded={!!isExpanded}
>
{isExpanded ? (
<LuShrink className="size-3.5" />
) : (📝 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.
Suggested change
| {onExpandToggle && ( | |
| <Tooltip> | |
| <TooltipTrigger asChild> | |
| <Button | |
| variant="ghost" | |
| size="icon" | |
| onClick={onExpandToggle} | |
| className="size-6 p-0" | |
| > | |
| {isExpanded ? ( | |
| <LuShrink className="size-3.5" /> | |
| ) : ( | |
| <LuExpand className="size-3.5" /> | |
| )} | |
| </Button> | |
| </TooltipTrigger> | |
| <TooltipContent side="top" showArrow={false}> | |
| {isExpanded ? "Collapse" : "Expand"} | |
| </TooltipContent> | |
| </Tooltip> | |
| {onExpandToggle && ( | |
| <Tooltip> | |
| <TooltipTrigger asChild> | |
| <Button | |
| variant="ghost" | |
| size="icon" | |
| onClick={onExpandToggle} | |
| className="size-6 p-0" | |
| aria-label={ | |
| isExpanded ? "Collapse changes view" : "Expand changes view" | |
| } | |
| aria-expanded={!!isExpanded} | |
| > | |
| {isExpanded ? ( | |
| <LuShrink className="size-3.5" /> | |
| ) : ( | |
| <LuExpand className="size-3.5" /> | |
| )} | |
| </Button> | |
| </TooltipTrigger> | |
| <TooltipContent side="top" showArrow={false}> | |
| {isExpanded ? "Collapse" : "Expand"} | |
| </TooltipContent> | |
| </Tooltip> |
🤖 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/WorkspaceView/RightSidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsx`
around lines 308 - 327, The expand toggle is an icon-only Button (rendered when
onExpandToggle is provided) and lacks accessible labeling/state; update the
Button in ChangesHeader so it includes an explicit aria-label (use the same text
as TooltipContent: isExpanded ? "Collapse" : "Expand") and set
aria-expanded={isExpanded} so screen readers get the current state; keep the
existing Tooltip/TooltipContent but ensure the Button element (the one with
variant="ghost" size="icon" onClick={onExpandToggle} and the icon children
LuShrink/LuExpand) receives these attributes.
Comment on lines
+109
to
+114
| const panelWidth = side === "left" ? leftPanelWidth : sidebarWidth; | ||
| const compactTabs = panelWidth < 250; | ||
| const showChangesTab = | ||
| !!worktreePath && tabPositions[RightSidebarTab.Changes] === side; | ||
| const showFilesTab = tabPositions[RightSidebarTab.Files] === side; | ||
| const oppositeSide: PanelSide = side === "left" ? "right" : "left"; |
Contributor
There was a problem hiding this comment.
Treat a split-out single tab as active on its own side.
When Changes and Files are docked to opposite sides, clicking Files makes rightSidebarTab === Files. This side then renders Changes as inactive/hidden even though there is no alternate tab to show here, so the pane can go blank and ChangesView stops polling. The per-side tab/button/content state needs to fall back to “active” when that side only has one visible tab.
Suggested fix
const showChangesTab =
!!worktreePath && tabPositions[RightSidebarTab.Changes] === side;
const showFilesTab = tabPositions[RightSidebarTab.Files] === side;
+ const isChangesVisible =
+ showChangesTab &&
+ (rightSidebarTab === RightSidebarTab.Changes || !showFilesTab);
+ const isFilesVisible =
+ showFilesTab &&
+ (rightSidebarTab === RightSidebarTab.Files || !showChangesTab);
const oppositeSide: PanelSide = side === "left" ? "right" : "left";
@@
<TabButton
- isActive={rightSidebarTab === RightSidebarTab.Changes}
+ isActive={isChangesVisible}
onClick={() => setRightSidebarTab(RightSidebarTab.Changes)}
icon={<LuGitCompareArrows className="size-3.5" />}
label="Changes"
@@
<TabButton
- isActive={rightSidebarTab === RightSidebarTab.Files}
+ isActive={isFilesVisible}
onClick={() => setRightSidebarTab(RightSidebarTab.Files)}
icon={<LuFile className="size-3.5" />}
label="Files"
@@
{showChangesTab && (
<div
- className={
- rightSidebarTab === RightSidebarTab.Changes
- ? "flex-1 min-h-0 flex flex-col overflow-hidden"
- : "hidden"
- }
+ className={
+ isChangesVisible
+ ? "flex-1 min-h-0 flex flex-col overflow-hidden"
+ : "hidden"
+ }
>
<ChangesView
onFileOpen={handleFileOpen}
isExpandedView={isExpanded}
- isActive={rightSidebarTab === RightSidebarTab.Changes}
+ isActive={isChangesVisible}
isExpanded={isExpanded}
onExpandToggle={handleExpandToggle}
/>
@@
{showFilesTab && (
<div
- className={
- rightSidebarTab === RightSidebarTab.Changes && showChangesTab
- ? "hidden"
- : "flex-1 min-h-0 flex flex-col overflow-hidden"
- }
+ className={
+ isFilesVisible
+ ? "flex-1 min-h-0 flex flex-col overflow-hidden"
+ : "hidden"
+ }
>
<FilesView />
</div>Also applies to: 212-267, 292-319
🤖 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/WorkspaceView/RightSidebar/index.tsx`
around lines 109 - 114, The current per-side visibility flags (showChangesTab
and showFilesTab) only check tabPositions and thus a side can end up with no
active tab when tabs are split; change the logic so each side computes its
visibleTabsOnSide (count of RightSidebarTab.Changes visible here considering
worktreePath and RightSidebarTab.Files) and treat a tab as active if it is
assigned to this side OR (visibleTabsOnSide === 1 and that tab is the only
visible one on this side). Update the entries that set showChangesTab and
showFilesTab (and any similar checks later in the file around the referenced
ranges) to use this rule so a single split-out tab becomes active on its own
side. Ensure you reference RightSidebarTab, tabPositions, rightSidebarTab, and
worktreePath in the new logic.
Comment on lines
283
to
287
| <TooltipContent side="bottom" showArrow={false}> | ||
| <HotkeyTooltipContent | ||
| label="Close sidebar" | ||
| label={side === "left" ? "Close left panel" : "Close sidebar"} | ||
| hotkeyId="TOGGLE_SIDEBAR" | ||
| /> |
Contributor
There was a problem hiding this comment.
Don't advertise the sidebar hotkey for the left-panel close action.
On the left side this button re-docks tabs and closes only that pane. TOGGLE_SIDEBAR still hides the whole sidebar, so the tooltip promises a shortcut that does not perform the labeled action.
Suggested fix
<TooltipContent side="bottom" showArrow={false}>
- <HotkeyTooltipContent
- label={side === "left" ? "Close left panel" : "Close sidebar"}
- hotkeyId="TOGGLE_SIDEBAR"
- />
+ {side === "left" ? (
+ "Close left panel"
+ ) : (
+ <HotkeyTooltipContent
+ label="Close sidebar"
+ hotkeyId="TOGGLE_SIDEBAR"
+ />
+ )}
</TooltipContent>📝 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.
Suggested change
| <TooltipContent side="bottom" showArrow={false}> | |
| <HotkeyTooltipContent | |
| label="Close sidebar" | |
| label={side === "left" ? "Close left panel" : "Close sidebar"} | |
| hotkeyId="TOGGLE_SIDEBAR" | |
| /> | |
| <TooltipContent side="bottom" showArrow={false}> | |
| {side === "left" ? ( | |
| "Close left panel" | |
| ) : ( | |
| <HotkeyTooltipContent | |
| label="Close sidebar" | |
| hotkeyId="TOGGLE_SIDEBAR" | |
| /> | |
| )} | |
| </TooltipContent> |
🤖 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/WorkspaceView/RightSidebar/index.tsx`
around lines 283 - 287, The tooltip currently shows the TOGGLE_SIDEBAR hotkey
for both sides but on the left the button only re-docks/ closes that pane;
update the HotkeyTooltipContent usage so when side === "left" it does not
advertise the TOGGLE_SIDEBAR hotkey: change the label to "Close left panel" and
remove or set hotkeyId to undefined/null for the left branch, and keep
hotkeyId="TOGGLE_SIDEBAR" only for the non-left (sidebar) branch; modify the
conditional around HotkeyTooltipContent to pick the correct label and hotkeyId
based on the side variable.
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.
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/.
Replace the individual project collapsibles in the settings sidebar with a single "Projects" link that opens a clean project list page. Selecting a project navigates to its settings. This scales much better when users have many active projects. - Replace per-project collapsibles in sidebar with single "Projects" nav item - Create /settings/projects route with a searchable project list - Each project shows name, color indicator, and repo path - Clicking a project navigates to /settings/project/$projectId/general
- Projects list page shows workspace count badges and workspace list per project (local branch + worktree workspaces) - Branch-only projects show "no worktrees" badge and single branch row - Project settings page gains "Workspaces" section between Project and Workspace Defaults, listing all workspaces with type badges - Rebased onto per-project-worktree-mode for all worktree UX changes
- Convert projects page to master-detail split view (project list on left, settings on right) instead of two-page drill-down - Always expand workspace/worktree sub-items under each project - Branch-only projects show collapsed (no sub-items, just project row) - Worktree items indented with left border line showing hierarchy - Move "Projects" into the "Editor & Workflow" sidebar group right after "Git & Worktrees" instead of its own separate section - Add workspace listing section to project settings detail view
- Replace color dots with ProjectThumbnail in the settings project list (shows favicon, GitHub avatar, or letter badge) - Add "Detect" button to project icon section that clears the current icon and re-discovers favicon from the project directory - Spinning icon while detection is in progress - Upload and Remove buttons still available alongside Detect
Add a new "file-tree" pane type to the react-mosaic pane system, so the file tree can be opened as a draggable, splittable pane alongside terminals, editors, and chat — instead of being confined to the sidebar. - Add "file-tree" to PaneType union and SplitPaneOptions - Add createFileTreePane() factory function - Update split operations in store to handle "file-tree" pane creation - Create FileTreePane component wrapping existing FilesView - Register pane routing in TabView's renderPane - Add "Split with File Tree" to pane context menu - Wire up context menu action in TabPane and TabContentContextMenu
react-dnd's connectDragSource requires native DOM elements, not React components. Wrapping PaneToolbarActions in a <div> fixes the error.
z3thon
force-pushed
the
feat/file-tree-pane
branch
from
dc52861 to
b758f1f
Compare
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/ProjectsSettings.tsx (1)
14-77:⚠️ Potential issue | 🟡 MinorRemove the unused
ProjectsSettingscomponent.The component is exported but not imported or used anywhere in the codebase.
SettingsSidebar.tsxonly rendersGeneralSettings. Consider deleting this file to eliminate dead code.🤖 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/components/SettingsSidebar/ProjectsSettings.tsx` around lines 14 - 77, The ProjectsSettings component is dead code (exported but unused); remove it to eliminate the dead file. Delete the ProjectsSettings function and its file (the exported ProjectsSettings symbol) and ensure SettingsSidebar (which only renders GeneralSettings) remains unchanged; run a project-wide search for "ProjectsSettings" to confirm no other imports exist before committing the deletion.
♻️ Duplicate comments (1)
apps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/components/ProjectSettings/ProjectSettings.tsx (1)
265-286:⚠️ Potential issue | 🟡 MinorMissing
awaiton final mutation may cause race condition.The
handleDisableWorktreesfunction awaits each workspace deletion but the finalupdateProject.mutate()is not awaited. Since the dialog closes immediately after calling this function (lines 869-871, 880-882), the worktree mode update might not complete before the UI state changes.Suggested fix
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({ + await updateProject.mutateAsync({ 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 265 - 286, The final updateProject.mutate call in handleDisableWorktrees is not awaited, causing a race with UI/dialog closing; change it to await the async variant (e.g., await updateProject.mutateAsync({ id: projectId, patch: { worktreeMode: "disabled" }})) or otherwise await the returned promise so the project update completes after the per-workspace delete/close loops (note related symbols: handleDisableWorktrees, deleteWorkspace.mutateAsync, closeWorkspaceMutation.mutateAsync, updateProject.mutate).
🧹 Nitpick comments (3)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileTreePane/FileTreePane.tsx (1)
6-6: Use renderer alias instead of deep relative import.Please switch this import to the configured alias form for consistency and easier refactors.
Suggested diff
-import { FilesView } from "../../../../RightSidebar/FilesView"; +import { FilesView } from "renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView";As per coding guidelines:
apps/desktop/**/*.{ts,tsx}: Use alias as defined intsconfig.jsonwhen possible.🤖 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/WorkspaceView/ContentView/TabsContent/TabView/FileTreePane/FileTreePane.tsx` at line 6, The import in FileTreePane.tsx currently uses a deep relative path to FilesView; replace the relative import ("../../../../RightSidebar/FilesView") with the configured renderer alias (e.g., import { FilesView } from "renderer/..." per tsconfig) so WorkspaceView/ContentView/TabsContent/TabView/FileTreePane references FilesView via the renderer alias for consistency and easier refactors.apps/desktop/src/lib/trpc/routers/projects/utils/favicon-discovery.ts (1)
37-45: Glob patterns with**may reduce match priority predictability.The
FAVICON_PATTERNSarray determines priority by order, butfast-globreturns matches sorted by the first matching pattern. Patterns like**/app/favicon.ico(line 37) could match before more specific patterns likeapp/favicon.ico(line 34) depending on directory structure.If strict priority is required, consider post-processing matches to re-sort by pattern index, or document that the current behavior is acceptable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/projects/utils/favicon-discovery.ts` around lines 37 - 45, FAVICON_PATTERNS uses glob patterns with "**" which can make match ordering unpredictable; after calling fast-glob (where you currently use FAVICON_PATTERNS), collect the matched file paths and re-sort them by the earliest index of the pattern that matched each path (i.e., build a map from pattern -> index, test each path against patterns in order to find its lowest index, then stable-sort matches by that index) so the final result strictly respects the intended priority order defined in FAVICON_PATTERNS.apps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/components/ProjectSettings/ProjectSettings.tsx (1)
489-509: Consider showing "Detect" button when no icon exists regardless of manual upload state.The "Detect" button is hidden when
iconManuallyUploadedis true, but if a user removes a manually uploaded icon (lines 522-537),iconManuallyUploadedbecomes false, allowing re-detection. However, if the user opens settings fresh without an icon and hasn't uploaded,iconManuallyUploadeddefaults tofalse(line 170), so the button shows correctly.The logic is correct, though the UX might benefit from always showing "Detect" when
!project.iconUrlregardless of the flag.🤖 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 489 - 509, The Detect button is currently gated by the iconManuallyUploaded flag; change the render condition to show the Detect button whenever the project has no icon (project.iconUrl is falsy) so users can always trigger detection regardless of iconManuallyUploaded; update the JSX branch that currently checks {!iconManuallyUploaded} to instead check {!project.iconUrl} (while keeping existing props/handlers: onClick={handleRefreshIcon}, disabled={discoverIcon.isPending || setProjectIcon.isPending}, and the LuRefreshCw icon class logic).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/ProjectsSettings.tsx`:
- Around line 14-77: The ProjectsSettings component is dead code (exported but
unused); remove it to eliminate the dead file. Delete the ProjectsSettings
function and its file (the exported ProjectsSettings symbol) and ensure
SettingsSidebar (which only renders GeneralSettings) remains unchanged; run a
project-wide search for "ProjectsSettings" to confirm no other imports exist
before committing the deletion.
---
Duplicate comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/project/`$projectId/components/ProjectSettings/ProjectSettings.tsx:
- Around line 265-286: The final updateProject.mutate call in
handleDisableWorktrees is not awaited, causing a race with UI/dialog closing;
change it to await the async variant (e.g., await updateProject.mutateAsync({
id: projectId, patch: { worktreeMode: "disabled" }})) or otherwise await the
returned promise so the project update completes after the per-workspace
delete/close loops (note related symbols: handleDisableWorktrees,
deleteWorkspace.mutateAsync, closeWorkspaceMutation.mutateAsync,
updateProject.mutate).
---
Nitpick comments:
In `@apps/desktop/src/lib/trpc/routers/projects/utils/favicon-discovery.ts`:
- Around line 37-45: FAVICON_PATTERNS uses glob patterns with "**" which can
make match ordering unpredictable; after calling fast-glob (where you currently
use FAVICON_PATTERNS), collect the matched file paths and re-sort them by the
earliest index of the pattern that matched each path (i.e., build a map from
pattern -> index, test each path against patterns in order to find its lowest
index, then stable-sort matches by that index) so the final result strictly
respects the intended priority order defined in FAVICON_PATTERNS.
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/project/`$projectId/components/ProjectSettings/ProjectSettings.tsx:
- Around line 489-509: The Detect button is currently gated by the
iconManuallyUploaded flag; change the render condition to show the Detect button
whenever the project has no icon (project.iconUrl is falsy) so users can always
trigger detection regardless of iconManuallyUploaded; update the JSX branch that
currently checks {!iconManuallyUploaded} to instead check {!project.iconUrl}
(while keeping existing props/handlers: onClick={handleRefreshIcon},
disabled={discoverIcon.isPending || setProjectIcon.isPending}, and the
LuRefreshCw icon class logic).
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileTreePane/FileTreePane.tsx`:
- Line 6: The import in FileTreePane.tsx currently uses a deep relative path to
FilesView; replace the relative import ("../../../../RightSidebar/FilesView")
with the configured renderer alias (e.g., import { FilesView } from
"renderer/..." per tsconfig) so
WorkspaceView/ContentView/TabsContent/TabView/FileTreePane references FilesView
via the renderer alias for consistency and easier refactors.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b428c57c-83aa-47ff-b97f-fd288b89d090
📒 Files selected for processing (26)
apps/desktop/src/lib/trpc/routers/projects/projects.tsapps/desktop/src/lib/trpc/routers/projects/utils/favicon-discovery.tsapps/desktop/src/renderer/react-query/projects/useOpenProject.tsxapps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/GeneralSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/ProjectsSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/SettingsSidebar.tsxapps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/components/ProjectSettings/ProjectSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/projects/page.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/SidebarDropZone/SidebarDropZone.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabContentContextMenu.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileTreePane/FileTreePane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileTreePane/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/TabPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/components/PaneContextMenuItems/PaneContextMenuItems.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/ChangesView.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceLayout/WorkspaceLayout.tsxapps/desktop/src/renderer/stores/sidebar-state.tsapps/desktop/src/renderer/stores/tabs/store.tsapps/desktop/src/renderer/stores/tabs/types.tsapps/desktop/src/renderer/stores/tabs/utils.tsapps/desktop/src/shared/tabs-types.ts
💤 Files with no reviewable changes (1)
- apps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/SettingsSidebar.tsx
✅ Files skipped from review due to trivial changes (3)
- apps/desktop/src/renderer/stores/tabs/types.ts
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileTreePane/index.ts
- apps/desktop/src/shared/tabs-types.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- apps/desktop/src/renderer/stores/tabs/utils.ts
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/index.tsx
- apps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/GeneralSettings.tsx
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/components/PaneContextMenuItems/PaneContextMenuItems.tsx
- apps/desktop/src/renderer/stores/tabs/store.ts
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabContentContextMenu.tsx
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsx
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceLayout/WorkspaceLayout.tsx
- apps/desktop/src/renderer/stores/sidebar-state.ts
- apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectSection.tsx
…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
…ings-list # Conflicts: # apps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/SettingsSidebar.tsx # apps/desktop/src/renderer/routes/_authenticated/settings/projects/page.tsx
- Wire onSplitWithFileTree in ChatPane so "Split with File Tree" context menu item appears on chat panes - Resolve merge conflicts with upstream (chat-mastra rename, drop zones)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/desktop/src/renderer/react-query/projects/useOpenProject.tsx (1)
239-244: Consider using async/await for consistency with other code paths.The dialog path at lines 226-229 uses
async/await, while this direct success path uses.then(). For consistency and slightly more defensive error handling, consider using the same pattern:♻️ Suggested refactor
if ("project" in result) { - maybePromptWorktreeChoice([result.project]).then(() => { - resolve(result.project); - }); + (async () => { + await maybePromptWorktreeChoice([result.project]); + resolve(result.project); + })(); return; }Or alternatively, make the entire
onSuccesscallback async (as done inopenNewat line 111).🤖 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 239 - 244, The direct success branch inside the onSuccess callback uses .then() for maybePromptWorktreeChoice([result.project]) while other paths use async/await; change this branch to use await by making the onSuccess callback async (or the specific handler async) and replace the .then(...) call with an awaited call to maybePromptWorktreeChoice([result.project]) wrapped in a try/catch so any errors are handled before calling resolve(result.project); mirror the error-handling style used in openNew to keep consistency.
🤖 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 346-356: The return object for an existing workspace (where you
spread `existing` and set `branch`, `lastOpenedAt`) is missing the
`initialCommands` field, causing an inconsistent response shape versus the
new-workspace branch which sets `initialCommands: null`; update the
existing-workspace return to include `initialCommands: null` (so the returned
object includes `workspace`, `worktreePath`, `projectId`, `isInitializing`,
`wasExisting`, and `initialCommands`) to match the new-workspace branch and keep
the API response shape uniform.
In `@apps/desktop/src/renderer/react-query/projects/useOpenProject.tsx`:
- Around line 155-208: openNewWithoutGit auto-initializes git but never calls
maybePromptWorktreeChoice (unlike openNew), so projects opened here won't show
the worktree mode prompt; update openNewWithoutGit to invoke
maybePromptWorktreeChoice for each project after it's opened/initiated (call
maybePromptWorktreeChoice(project) for successes and for projects returned by
initGitAndOpen) and ensure maybePromptWorktreeChoice is added to the useCallback
dependency array along with initGitAndOpen, openNewMutation, and utils; if
omission was intentional, add a brief comment in openNewWithoutGit explaining
why the prompt is skipped.
---
Nitpick comments:
In `@apps/desktop/src/renderer/react-query/projects/useOpenProject.tsx`:
- Around line 239-244: The direct success branch inside the onSuccess callback
uses .then() for maybePromptWorktreeChoice([result.project]) while other paths
use async/await; change this branch to use await by making the onSuccess
callback async (or the specific handler async) and replace the .then(...) call
with an awaited call to maybePromptWorktreeChoice([result.project]) wrapped in a
try/catch so any errors are handled before calling resolve(result.project);
mirror the error-handling style used in openNew to keep consistency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c58f146f-0970-403a-82a3-914f728674dd
📒 Files selected for processing (5)
apps/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/components/WorktreeChoiceDialog/ConnectedWorktreeChoiceDialog.tsxapps/desktop/src/renderer/react-query/projects/useOpenProject.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx
- apps/desktop/src/renderer/components/WorktreeChoiceDialog/ConnectedWorktreeChoiceDialog.tsx
- apps/desktop/src/lib/trpc/routers/settings/index.ts
Comment on lines
+346
to
+356
| return { | ||
| workspace: { | ||
| ...existing, | ||
| branch, | ||
| lastOpenedAt: Date.now(), | ||
| }, | ||
| worktreePath: project.mainRepoPath, | ||
| projectId: project.id, | ||
| isInitializing: false, | ||
| wasExisting: true, | ||
| }; |
Contributor
There was a problem hiding this comment.
Inconsistent return shape: initialCommands missing for existing workspace path.
The existing workspace return (lines 346-356) omits initialCommands, while the new workspace return (lines 387-394) includes initialCommands: null. This inconsistency could cause issues if the client expects a uniform return shape.
Proposed fix: Add `initialCommands: null` for consistency
return {
workspace: {
...existing,
branch,
lastOpenedAt: Date.now(),
},
worktreePath: project.mainRepoPath,
projectId: project.id,
+ initialCommands: null,
isInitializing: false,
wasExisting: true,
};📝 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.
Suggested change
| return { | |
| workspace: { | |
| ...existing, | |
| branch, | |
| lastOpenedAt: Date.now(), | |
| }, | |
| worktreePath: project.mainRepoPath, | |
| projectId: project.id, | |
| isInitializing: false, | |
| wasExisting: true, | |
| }; | |
| return { | |
| workspace: { | |
| ...existing, | |
| branch, | |
| lastOpenedAt: Date.now(), | |
| }, | |
| worktreePath: project.mainRepoPath, | |
| projectId: project.id, | |
| initialCommands: null, | |
| isInitializing: false, | |
| wasExisting: true, | |
| }; |
🤖 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 346 - 356, The return object for an existing workspace (where you spread
`existing` and set `branch`, `lastOpenedAt`) is missing the `initialCommands`
field, causing an inconsistent response shape versus the new-workspace branch
which sets `initialCommands: null`; update the existing-workspace return to
include `initialCommands: null` (so the returned object includes `workspace`,
`worktreePath`, `projectId`, `isInitializing`, `wasExisting`, and
`initialCommands`) to match the new-workspace branch and keep the API response
shape uniform.
Comment on lines
+155
to
+208
| /** Opens a folder picker and auto-initializes git if needed (no dialog). */ | ||
| const openNewWithoutGit = useCallback((): Promise<Project[]> => { | ||
| return new Promise((resolve) => { | ||
| openNewMutation.mutate(undefined, { | ||
| onSuccess: async (result) => { | ||
| if (result.canceled) { | ||
| resolve([]); | ||
| return; | ||
| } | ||
|
|
||
| if ("error" in result) { | ||
| resolve([]); | ||
| return; | ||
| } | ||
|
|
||
| if ("results" in result) { | ||
| const { successes, needsGitInit } = processOpenNewResults({ | ||
| results: result.results, | ||
| }); | ||
|
|
||
| const allProjects = successes.map((s) => s.project); | ||
|
|
||
| // Auto-init git for any folders that need it (skip the dialog) | ||
| for (const item of needsGitInit) { | ||
| try { | ||
| const initiated = await initGitAndOpen.mutateAsync({ | ||
| path: item.selectedPath, | ||
| }); | ||
| allProjects.push(initiated.project); | ||
| } catch (error) { | ||
| console.error( | ||
| "[useOpenProject] Failed to auto-init git:", | ||
| item.selectedPath, | ||
| error, | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| if (allProjects.length > 0) { | ||
| await utils.projects.getRecents.invalidate(); | ||
| } | ||
|
|
||
| resolve(allProjects); | ||
| return; | ||
| } | ||
|
|
||
| resolve([]); | ||
| }, | ||
| onError: () => { | ||
| resolve([]); | ||
| }, | ||
| }); | ||
| }); | ||
| }, [initGitAndOpen, openNewMutation, utils]); |
Contributor
There was a problem hiding this comment.
Missing maybePromptWorktreeChoice call for consistency.
openNewWithoutGit auto-initializes git but doesn't call maybePromptWorktreeChoice, unlike openNew which prompts for worktree choice on all opened projects. This means projects opened via this path won't get the worktree mode configuration dialog.
If this is intentional (e.g., this function is used in contexts where worktree choice isn't needed), please add a comment clarifying that. Otherwise, consider adding the call:
🔧 Proposed fix
if (allProjects.length > 0) {
await utils.projects.getRecents.invalidate();
}
+ await maybePromptWorktreeChoice(allProjects);
resolve(allProjects);
return;And update the dependency array:
- }, [initGitAndOpen, openNewMutation, utils]);
+ }, [initGitAndOpen, maybePromptWorktreeChoice, openNewMutation, utils]);📝 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.
Suggested change
| /** Opens a folder picker and auto-initializes git if needed (no dialog). */ | |
| const openNewWithoutGit = useCallback((): Promise<Project[]> => { | |
| return new Promise((resolve) => { | |
| openNewMutation.mutate(undefined, { | |
| onSuccess: async (result) => { | |
| if (result.canceled) { | |
| resolve([]); | |
| return; | |
| } | |
| if ("error" in result) { | |
| resolve([]); | |
| return; | |
| } | |
| if ("results" in result) { | |
| const { successes, needsGitInit } = processOpenNewResults({ | |
| results: result.results, | |
| }); | |
| const allProjects = successes.map((s) => s.project); | |
| // Auto-init git for any folders that need it (skip the dialog) | |
| for (const item of needsGitInit) { | |
| try { | |
| const initiated = await initGitAndOpen.mutateAsync({ | |
| path: item.selectedPath, | |
| }); | |
| allProjects.push(initiated.project); | |
| } catch (error) { | |
| console.error( | |
| "[useOpenProject] Failed to auto-init git:", | |
| item.selectedPath, | |
| error, | |
| ); | |
| } | |
| } | |
| if (allProjects.length > 0) { | |
| await utils.projects.getRecents.invalidate(); | |
| } | |
| resolve(allProjects); | |
| return; | |
| } | |
| resolve([]); | |
| }, | |
| onError: () => { | |
| resolve([]); | |
| }, | |
| }); | |
| }); | |
| }, [initGitAndOpen, openNewMutation, utils]); | |
| /** Opens a folder picker and auto-initializes git if needed (no dialog). */ | |
| const openNewWithoutGit = useCallback((): Promise<Project[]> => { | |
| return new Promise((resolve) => { | |
| openNewMutation.mutate(undefined, { | |
| onSuccess: async (result) => { | |
| if (result.canceled) { | |
| resolve([]); | |
| return; | |
| } | |
| if ("error" in result) { | |
| resolve([]); | |
| return; | |
| } | |
| if ("results" in result) { | |
| const { successes, needsGitInit } = processOpenNewResults({ | |
| results: result.results, | |
| }); | |
| const allProjects = successes.map((s) => s.project); | |
| // Auto-init git for any folders that need it (skip the dialog) | |
| for (const item of needsGitInit) { | |
| try { | |
| const initiated = await initGitAndOpen.mutateAsync({ | |
| path: item.selectedPath, | |
| }); | |
| allProjects.push(initiated.project); | |
| } catch (error) { | |
| console.error( | |
| "[useOpenProject] Failed to auto-init git:", | |
| item.selectedPath, | |
| error, | |
| ); | |
| } | |
| } | |
| if (allProjects.length > 0) { | |
| await utils.projects.getRecents.invalidate(); | |
| } | |
| await maybePromptWorktreeChoice(allProjects); | |
| resolve(allProjects); | |
| return; | |
| } | |
| resolve([]); | |
| }, | |
| onError: () => { | |
| resolve([]); | |
| }, | |
| }); | |
| }); | |
| }, [initGitAndOpen, maybePromptWorktreeChoice, openNewMutation, utils]); |
🤖 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 155 - 208, openNewWithoutGit auto-initializes git but never calls
maybePromptWorktreeChoice (unlike openNew), so projects opened here won't show
the worktree mode prompt; update openNewWithoutGit to invoke
maybePromptWorktreeChoice for each project after it's opened/initiated (call
maybePromptWorktreeChoice(project) for successes and for projects returned by
initGitAndOpen) and ensure maybePromptWorktreeChoice is added to the useCallback
dependency array along with initGitAndOpen, openNewMutation, and utils; if
omission was intentional, add a brief comment in openNewWithoutGit explaining
why the prompt is skipped.
…ttons Replace complex auto-move sidebar logic with simple independent panels: - Add isLeftPanelOpen/isRightPanelOpen to sidebar store (replaces single isSidebarOpen for panel visibility) - Add toggleLeftPanel()/toggleRightPanel() methods - Add PanelToggleButtons component (LuPanelLeft/LuPanelRight icons) always visible in content header, highlighted when panel is open - Simplify handleClosePanel to just toggle the panel, no tab moving - Each panel's tabs stay where they are when opened/closed - Backward compat: isSidebarOpen/toggleSidebar still work (map to right) - Persist migration v1→v2 initializes new panel states from legacy
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
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/PanelToggleButtons/PanelToggleButtons.tsx">
<violation number="1" location="apps/desktop/src/renderer/screens/main/components/PanelToggleButtons/PanelToggleButtons.tsx:43">
P2: Left panel tooltip shows a hotkey ID that triggers right-sidebar toggle, creating incorrect shortcut guidance.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| <TooltipContent side="bottom"> | ||
| <HotkeyTooltipContent | ||
| label={isLeftPanelOpen ? "Close left panel" : "Open left panel"} | ||
| hotkeyId="TOGGLE_SIDEBAR" |
Contributor
There was a problem hiding this comment.
P2: Left panel tooltip shows a hotkey ID that triggers right-sidebar toggle, creating incorrect shortcut guidance.
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/PanelToggleButtons/PanelToggleButtons.tsx, line 43:
<comment>Left panel tooltip shows a hotkey ID that triggers right-sidebar toggle, creating incorrect shortcut guidance.</comment>
<file context>
@@ -9,31 +17,62 @@ export function PanelToggleButtons() {
+ <TooltipContent side="bottom">
+ <HotkeyTooltipContent
+ label={isLeftPanelOpen ? "Close left panel" : "Open left panel"}
+ hotkeyId="TOGGLE_SIDEBAR"
+ />
+ </TooltipContent>
</file context>
# Conflicts: # apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/TopBar.tsx
…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
|
i need this feature BAD |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Why / Context
Users want the file tree accessible alongside their terminal without replacing the sidebar. Additionally, having sidebar panels locked to the right side limits layout flexibility — users with wide monitors may prefer files on the left and changes on the right, or vice versa.
How It Works
File Tree Pane
Movable Sidebar Panels
tabPositions)ResizablePanelcomponents as neededClose Behavior
isSidebarOpenwhich controls all panelsExpand Button Relocation
ChangesHeadertoolbar (the row with base branch, stash, refresh, etc.)Manual QA Checklist
File Tree Pane
Movable Panels
Close Behavior
Expand Button
Testing
bun run typecheck— passesbun x @biomejs/biome check— cleanFiles Changed
FileTreePane.tsx— new pane component + drag fix (div wrapper)sidebar-state.ts—tabPositions,leftPanelWidth,setTabPositionWorkspaceLayout.tsx— conditional left/right panel renderingRightSidebar/index.tsx— side prop, tab filtering, context menus, close logicChangesView.tsx— pass expand props throughChangesHeader.tsx— render expand/collapse buttonTabPane.tsx,TabView/index.tsx— wire up file tree pane typetabs/store.ts,tabs/types.ts,tabs/utils.ts— file-tree pane supportSummary by CodeRabbit
Release Notes
New Features
Improvements