Repository navigation
feat(desktop): v2 workspace right sidebar with file tree - #3149
Merged
Merged
Conversation
Add a tabbed right sidebar to the v2 workspace with: - SidebarHeader with pill-style tab bar (Files, Changes, Checks) - FilesTab with recursive file tree, sticky folder headers, JS-based hover tracking, context menus, collapse/refresh actions - Stub tabs for Changes and Checks (coming soon) - Wire sidebar toggle, file selection, and quick open search Replaces the old flat file list + search toolbar with a proper tree view using the existing useFileTree hook from workspace-client.
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughReplaces the old RightSidebar file-tree/search with a new WorkspaceSidebar tab shell. Adds FilesTab (full file-tree explorer with create/rename/delete, context menus, refresh, hover/selection, reveal/scroll), and placeholder "changes" and "checks" tabs; search exposed as an optional header action. Several old toolbar/search components and hooks were removed. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SidebarHeader as SidebarHeader (UI)
participant WorkspaceSidebar as WorkspaceSidebar (container)
participant FilesTab as FilesTab (tree UI)
participant TRPC as Workspace TRPC
participant FS as Filesystem
User->>SidebarHeader: Click "files" tab or Search action
SidebarHeader->>WorkspaceSidebar: onTabChange / onSearch
WorkspaceSidebar->>FilesTab: Render or focus FilesTab
User->>FilesTab: Create/Rename/Delete/Select node
FilesTab->>TRPC: Call mutation (create/move/delete/read)
TRPC->>FS: Execute filesystem operation
FS-->>TRPC: Operation result
TRPC-->>FilesTab: Mutation response
FilesTab->>FilesTab: Invalidate/refresh tree (useFileTree)
FilesTab->>TRPC: Fetch updated tree
TRPC-->>FilesTab: Updated tree data
FilesTab-->>User: Update UI (reveal/scroll/selection)
sequenceDiagram
participant User
participant FilesTab
participant ContextMenu
participant Electron as Electron Client
participant Clipboard
User->>FilesTab: Right-click node
FilesTab->>ContextMenu: Show File/Folder menu
User->>ContextMenu: Choose "Reveal in Finder"
ContextMenu->>Electron: openInFinder.mutate(absolutePath)
Electron-->>ContextMenu: Result
User->>ContextMenu: Choose "Copy Path"
ContextMenu->>Clipboard: navigator.clipboard.writeText(path)
ContextMenu-->>User: toast.success
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 docstrings
🧪 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 |
Contributor
Greptile SummaryThis PR replaces the old flat file list in the v2 workspace right sidebar with a fully recursive tree view, adds a tabbed Key observations:
Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
Page["page.tsx\nV2WorkspacePage"] --> RS["RightSidebar"]
RS --> SH["SidebarHeader\n(Files / Changes / Checks tabs)"]
RS --> FT["FilesTab\n(active=files)"]
RS --> CS["'Coming soon'\n(changes/checks)"]
FT --> WT["workspaceTrpc\n.workspace.get"]
FT --> UFT["useFileTree\n(rootEntries, toggle, reveal, collapseAll)"]
FT --> UFSEB["useWorkspaceFsEventBridge"]
FT --> UFSE["useWorkspaceFsEvents\n(invalidate search cache)"]
FT --> TN["TreeNode (recursive)"]
TN --> WFTI["WorkspaceFilesTreeItem\n(sticky folder header / file row)"]
WFTI --> FCM["FileContextMenu\n⚠ no onClick handlers"]
WFTI --> FOLCM["FolderContextMenu\n⚠ no onClick handlers"]
SH --> Actions["Per-tab actions slot\n(Search icon for Files tab)"]
Actions --> CP["CommandPalette\n(onSearch)"]
Reviews (1): Last reviewed commit: "feat(desktop): v2 workspace right sideba..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+11
to
+38
| <ContextMenuItem> | ||
| Open to the Side | ||
| <ContextMenuShortcut>⌥↵</ContextMenuShortcut> | ||
| </ContextMenuItem> | ||
| <ContextMenuSeparator /> | ||
| <ContextMenuItem> | ||
| Reveal in Finder | ||
| <ContextMenuShortcut>⌥⌘R</ContextMenuShortcut> | ||
| </ContextMenuItem> | ||
| <ContextMenuItem>Open in Integrated Terminal</ContextMenuItem> | ||
| <ContextMenuSeparator /> | ||
| <ContextMenuItem> | ||
| Copy Path | ||
| <ContextMenuShortcut>⌥⌘C</ContextMenuShortcut> | ||
| </ContextMenuItem> | ||
| <ContextMenuItem> | ||
| Copy Relative Path | ||
| <ContextMenuShortcut>⌥⇧⌘C</ContextMenuShortcut> | ||
| </ContextMenuItem> | ||
| <ContextMenuSeparator /> | ||
| <ContextMenuItem> | ||
| Rename... | ||
| <ContextMenuShortcut>↵</ContextMenuShortcut> | ||
| </ContextMenuItem> | ||
| <ContextMenuItem className="text-destructive focus:text-destructive"> | ||
| Delete | ||
| <ContextMenuShortcut>⌘⌫</ContextMenuShortcut> | ||
| </ContextMenuItem> |
Contributor
There was a problem hiding this comment.
All ContextMenuItem elements in FileContextMenu (and similarly in FolderContextMenu) render visible, interactive-looking menu entries — "Open to the Side", "Reveal in Finder", "Copy Path", "Rename...", "Delete", etc. — but none have onClick handlers attached. Clicking any item silently does nothing.
If these are intentional stubs for follow-up PRs, adding a comment or a no-op handler with a // TODO makes the intent clear. If "Reveal in Finder" / "Copy Path" are expected to work now (they're straightforward IPC calls), they should be wired up before shipping. At minimum, the "Delete" item is visually destructive (text-destructive) and doing nothing on click may surprise users.
Same concern applies to FolderContextMenu.tsx.
Comment on lines
+200
to
+215
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <Button variant="ghost" size="icon" className="size-5"> | ||
| <FilePlus className="size-3" /> | ||
| </Button> | ||
| </TooltipTrigger> | ||
| <TooltipContent side="bottom">New File</TooltipContent> | ||
| </Tooltip> | ||
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <Button variant="ghost" size="icon" className="size-5"> | ||
| <FolderPlus className="size-3" /> | ||
| </Button> | ||
| </TooltipTrigger> | ||
| <TooltipContent side="bottom">New Folder</TooltipContent> | ||
| </Tooltip> |
Contributor
There was a problem hiding this comment.
onClick handlers
Both the FilePlus and FolderPlus buttons in the workspace root header row are rendered without an onClick prop. Hovering shows the tooltip ("New File" / "New Folder") but clicking does nothing. Users who try to create files/folders from this UI will get no feedback.
Consider either wiring up the handlers or disabling/hiding these buttons until the functionality is implemented in a follow-up PR.
| workspaceId, | ||
| workspaceName, | ||
| }: FilesTabProps) { | ||
| const [_isRefreshing, setIsRefreshing] = useState(false); |
Contributor
There was a problem hiding this comment.
_isRefreshing state is declared but never consumed
_isRefreshing is set to true while fileTree.refreshAll() is running (line 154) but is never read back in the render — the RefreshCw button has no aria-busy, spinner, or disabled state tied to it. The user therefore gets no visual feedback that a refresh is in progress.
Suggested change
| const [_isRefreshing, setIsRefreshing] = useState(false); | |
| const [isRefreshing, setIsRefreshing] = useState(false); |
Then use isRefreshing to show a loading indicator on the refresh button (e.g. disabled={isRefreshing} or a spinning icon class).
Comment on lines
+53
to
+58
| ...(isFolder | ||
| ? { | ||
| position: "sticky" as const, | ||
| top: depth * rowHeight, | ||
| zIndex: 10 - depth, | ||
| } |
Contributor
There was a problem hiding this comment.
zIndex: 10 - depth produces 0 at depth 10 and negative values beyond that. A negative z-index would cause those sticky folder headers to render beneath surrounding non-sticky content, breaking the cascading-headers effect for deeply nested workspaces.
A more robust approach is to invert the formula so deeper items are always above a known baseline:
Suggested change
| ...(isFolder | |
| ? { | |
| position: "sticky" as const, | |
| top: depth * rowHeight, | |
| zIndex: 10 - depth, | |
| } | |
| zIndex: Math.max(1, 10 - depth), |
Or clamp the depth used in the formula so it never goes below 1.
![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: 3
🧹 Nitpick comments (4)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsx (1)
5-5: Avoid coupling the v2 explorer to the legacyFilesViewsubtree.At Line 5, this new sidebar row imports
FileIconfrom the oldWorkspaceView/RightSidebar/FilesViewpath, andFilesTab.tsxdoes the same for tree constants. That makes the new explorer depend on a legacy screen implementation. Prefer moving these primitives into this feature tree or a shared explorer module.As per coding guidelines, "Co-locate component dependencies (utils, hooks, constants, config, stories) next to the file using them."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/RightSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsx at line 5, The WorkspaceFilesTreeItem import of FileIcon (and FilesTab.tsx's import of tree constants) couples the new v2 explorer to the legacy FilesView subtree; extract the reusable primitive(s) (e.g., FileIcon and the tree constants) out of the legacy path into the v2 explorer feature folder or a small shared explorer module, update WorkspaceFilesTreeItem.tsx and FilesTab.tsx to import from the new colocated/shared module, and ensure any related utils/hooks/constants are moved alongside and exported so the new explorer no longer depends on renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView.apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/RightSidebar.tsx (2)
18-42: Consider extractingIconButtonto a shared location.This
IconButtonpattern (ghost button + tooltip) is likely reusable across the codebase. For now, it's fine co-located here, but if you find yourself duplicating this in other sidebar components, consider extracting it to a shared UI component.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/RightSidebar/RightSidebar.tsx around lines 18 - 42, The IconButton component duplicates a common "ghost button + tooltip" pattern and should be extracted for reuse; locate the IconButton function in RightSidebar (the IconButton component that accepts props icon: Icon, tooltip, onClick) and move it into a shared UI module (e.g., a new/shared IconButton or TooltipIcon component), export it, and replace the local definition with an import; ensure the shared component preserves the same props/signature (React.ComponentType<{ className?: string }>, tooltip: string, onClick?: () => void) and usage of Tooltip, TooltipTrigger, TooltipContent and Button so existing callers (including RightSidebar) keep the same behavior.
62-68: Optional: Derive tab IDs from a const array for type safety.The cast
id as SidebarTabis safe with the current hardcoded tabs but could be eliminated by deriving the type from a const array.🔧 Type-safe approach
const SIDEBAR_TABS = [ { id: "files", label: "All files" }, { id: "changes", label: "Changes" }, { id: "checks", label: "Checks" }, ] as const; type SidebarTab = (typeof SIDEBAR_TABS)[number]["id"]; // Then use SIDEBAR_TABS in SidebarHeader and remove the cast🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/RightSidebar/RightSidebar.tsx around lines 62 - 68, Introduce a const array of tabs and derive the SidebarTab type from it to remove the unsafe cast: create SIDEBAR_TABS as a readonly const with entries matching the current tabs ({ id: "files", label: "All files" }, etc.), define type SidebarTab = (typeof SIDEBAR_TABS)[number]["id"], then pass SIDEBAR_TABS to the SidebarHeader/tabs prop and update onTabChange to call setActiveTab(id) without the `as SidebarTab` cast; update any usages of the previous hardcoded tabs or SidebarTab type to use the new SIDEBAR_TABS and derived SidebarTab.apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/SidebarHeader/SidebarHeader.tsx (1)
28-44: Consider adding accessibility attributes for tab semantics.The tab buttons work visually but lack semantic ARIA attributes that assist screen readers. Adding
role="tab",aria-selected, and optionallytabIndexwould improve accessibility.♿ Proposed accessibility enhancement
{tabs.map((tab) => ( <button key={tab.id} type="button" + role="tab" + aria-selected={activeTab === tab.id} onClick={() => onTabChange(tab.id)} className={cn(You may also want to wrap the tab list with
role="tablist"on line 26.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/RightSidebar/components/SidebarHeader/SidebarHeader.tsx around lines 28 - 44, The tab buttons rendered in the SidebarHeader lack ARIA tab semantics; update the button inside the tab rendering loop (the element using onTabChange and key={tab.id}) to include role="tab", aria-selected={activeTab === tab.id}, and a suitable tabIndex (0 when selected, -1 when not) so screen readers can identify selected state, and ensure the parent container that holds these buttons is marked role="tablist" to complete the tab semantics.
🤖 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/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/RightSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsx:
- Line 83: The context menus are rendered without the clicked node or action
handlers; update the JSX that mounts FolderContextMenu/FileContextMenu to pass
the clicked node and its absolute path (e.g., node and node.absolutePath) plus
handlers for rename/delete/reveal/copy (suggest prop names like onRename,
onDelete, onReveal, onCopy). Then update FolderContextMenu and FileContextMenu
prop types/consumers to accept these props and call the provided handlers with
the node or path when menu items are selected so actions operate on the
right-clicked row.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/RightSidebar/components/FilesTab/FilesTab.tsx:
- Around line 199-240: The explorer toolbar is only revealed on mouse hover and
its icon-only Buttons lack accessible names and have active but unimplemented
actions; update the toolbar container in FilesTab.tsx to also reveal on keyboard
focus by adding the group-focus-within:opacity-100 (alongside
group-hover:opacity-100) so keyboard users can tab into it, add explicit
aria-label attributes to each icon Button (e.g., Buttons wrapped by
Tooltip/TooltipTrigger for New File, New Folder, Refresh, Collapse All) so
screen readers announce them, and mark the unfinished actions (the New File and
New Folder Buttons) as disabled (or remove their tabIndex/put aria-hidden) until
their handlers are implemented; keep the existing handlers like handleRefresh
and fileTree.collapseAll as interactive and accessible.
- Around line 109-151: The effect currently advances prevSelectedRef.current
unconditionally, causing a pre-mark of selectedFilePath before rootPath is
available; change it so prevSelectedRef.current is only updated when we've
actually handled the selection: move the assignment of prevSelectedRef.current =
selectedFilePath inside the branch that runs fileTree.reveal (so it’s advanced
after/when reveal is attempted), and also ensure you clear or set
prevSelectedRef.current appropriately when selectedFilePath is falsy (so we
don't skip future reveals). Update the useEffect that references
selectedFilePath, rootPath, prevSelectedRef and fileTree.reveal accordingly.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/RightSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsx:
- Line 5: The WorkspaceFilesTreeItem import of FileIcon (and FilesTab.tsx's
import of tree constants) couples the new v2 explorer to the legacy FilesView
subtree; extract the reusable primitive(s) (e.g., FileIcon and the tree
constants) out of the legacy path into the v2 explorer feature folder or a small
shared explorer module, update WorkspaceFilesTreeItem.tsx and FilesTab.tsx to
import from the new colocated/shared module, and ensure any related
utils/hooks/constants are moved alongside and exported so the new explorer no
longer depends on
renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/RightSidebar/components/SidebarHeader/SidebarHeader.tsx:
- Around line 28-44: The tab buttons rendered in the SidebarHeader lack ARIA tab
semantics; update the button inside the tab rendering loop (the element using
onTabChange and key={tab.id}) to include role="tab", aria-selected={activeTab
=== tab.id}, and a suitable tabIndex (0 when selected, -1 when not) so screen
readers can identify selected state, and ensure the parent container that holds
these buttons is marked role="tablist" to complete the tab semantics.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/RightSidebar/RightSidebar.tsx:
- Around line 18-42: The IconButton component duplicates a common "ghost button
+ tooltip" pattern and should be extracted for reuse; locate the IconButton
function in RightSidebar (the IconButton component that accepts props icon:
Icon, tooltip, onClick) and move it into a shared UI module (e.g., a new/shared
IconButton or TooltipIcon component), export it, and replace the local
definition with an import; ensure the shared component preserves the same
props/signature (React.ComponentType<{ className?: string }>, tooltip: string,
onClick?: () => void) and usage of Tooltip, TooltipTrigger, TooltipContent and
Button so existing callers (including RightSidebar) keep the same behavior.
- Around line 62-68: Introduce a const array of tabs and derive the SidebarTab
type from it to remove the unsafe cast: create SIDEBAR_TABS as a readonly const
with entries matching the current tabs ({ id: "files", label: "All files" },
etc.), define type SidebarTab = (typeof SIDEBAR_TABS)[number]["id"], then pass
SIDEBAR_TABS to the SidebarHeader/tabs prop and update onTabChange to call
setActiveTab(id) without the `as SidebarTab` cast; update any usages of the
previous hardcoded tabs or SidebarTab type to use the new SIDEBAR_TABS and
derived SidebarTab.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 268eb3a8-1789-47f8-83c2-f8634365db3e
📒 Files selected for processing (19)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/RightSidebar.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/FilesTab.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/FileContextMenu/FileContextMenu.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/FileContextMenu/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/FolderContextMenu/FolderContextMenu.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/FolderContextMenu/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/SidebarHeader/SidebarHeader.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/SidebarHeader/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/WorkspaceFilesSearchResultItem/WorkspaceFilesSearchResultItem.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/WorkspaceFilesSearchResultItem/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/WorkspaceFilesToolbar/WorkspaceFilesToolbar.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/WorkspaceFilesToolbar/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/hooks/useWorkspaceFileSearch/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/hooks/useWorkspaceFileSearch/useWorkspaceFileSearch.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsx
💤 Files with no reviewable changes (7)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/WorkspaceFilesSearchResultItem/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/WorkspaceFilesToolbar/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/hooks/useWorkspaceFileSearch/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/hooks/useWorkspaceFileSearch/useWorkspaceFileSearch.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsx
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/WorkspaceFilesToolbar/WorkspaceFilesToolbar.tsx
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/WorkspaceFilesSearchResultItem/WorkspaceFilesSearchResultItem.tsx
Comment on lines
+109
to
+151
| const prevSelectedRef = useRef(selectedFilePath); | ||
|
|
||
| const updateHoverFromPoint = useCallback((x: number, y: number) => { | ||
| const el = document.elementFromPoint(x, y)?.closest("[data-filepath]"); | ||
| setHoveredPath(el?.getAttribute("data-filepath") ?? null); | ||
| }, []); | ||
|
|
||
| const handleMouseMove = useCallback( | ||
| (e: React.MouseEvent) => { | ||
| lastMousePos.current = { x: e.clientX, y: e.clientY }; | ||
| updateHoverFromPoint(e.clientX, e.clientY); | ||
| }, | ||
| [updateHoverFromPoint], | ||
| ); | ||
|
|
||
| const handleScroll = useCallback(() => { | ||
| if (lastMousePos.current) { | ||
| updateHoverFromPoint(lastMousePos.current.x, lastMousePos.current.y); | ||
| } | ||
| }, [updateHoverFromPoint]); | ||
|
|
||
| const handleMouseLeave = useCallback(() => { | ||
| lastMousePos.current = null; | ||
| setHoveredPath(null); | ||
| }, []); | ||
|
|
||
| useEffect(() => { | ||
| if ( | ||
| selectedFilePath && | ||
| selectedFilePath !== prevSelectedRef.current && | ||
| rootPath | ||
| ) { | ||
| void fileTree.reveal(selectedFilePath).then(() => { | ||
| requestAnimationFrame(() => { | ||
| const el = scrollContainerRef.current?.querySelector( | ||
| `[data-filepath="${CSS.escape(selectedFilePath)}"]`, | ||
| ); | ||
| el?.scrollIntoView({ block: "center" }); | ||
| }); | ||
| }); | ||
| } | ||
| prevSelectedRef.current = selectedFilePath; | ||
| }, [selectedFilePath, rootPath, fileTree]); |
Contributor
There was a problem hiding this comment.
Don't mark a selection as "seen" before the tree can reveal it.
At Lines 137-150, prevSelectedRef is advanced even when rootPath is still empty. If selectedFilePath is already set when FilesTab mounts, the first pass marks it as handled, and the later render with a resolved worktree skips fileTree.reveal(). The selected file then never expands/scrolls into view.
🛠 Proposed fix
- const prevSelectedRef = useRef(selectedFilePath);
+ const prevSelectedRef = useRef<string | undefined>(undefined);
useEffect(() => {
- if (
- selectedFilePath &&
- selectedFilePath !== prevSelectedRef.current &&
- rootPath
- ) {
+ if (!selectedFilePath) {
+ prevSelectedRef.current = undefined;
+ return;
+ }
+
+ if (
+ selectedFilePath &&
+ selectedFilePath !== prevSelectedRef.current &&
+ rootPath
+ ) {
void fileTree.reveal(selectedFilePath).then(() => {
+ prevSelectedRef.current = selectedFilePath;
requestAnimationFrame(() => {
const el = scrollContainerRef.current?.querySelector(
`[data-filepath="${CSS.escape(selectedFilePath)}"]`,
);
el?.scrollIntoView({ block: "center" });
});
});
}
- prevSelectedRef.current = selectedFilePath;
}, [selectedFilePath, rootPath, fileTree]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/RightSidebar/components/FilesTab/FilesTab.tsx
around lines 109 - 151, The effect currently advances prevSelectedRef.current
unconditionally, causing a pre-mark of selectedFilePath before rootPath is
available; change it so prevSelectedRef.current is only updated when we've
actually handled the selection: move the assignment of prevSelectedRef.current =
selectedFilePath inside the branch that runs fileTree.reveal (so it’s advanced
after/when reveal is attempted), and also ensure you clear or set
prevSelectedRef.current appropriately when selectedFilePath is falsy (so we
don't skip future reveals). Update the useEffect that references
selectedFilePath, rootPath, prevSelectedRef and fileTree.reveal accordingly.
Comment on lines
+199
to
+240
| <div className="flex items-center gap-0.5 opacity-0 group-hover:opacity-100 transition-opacity"> | ||
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <Button variant="ghost" size="icon" className="size-5"> | ||
| <FilePlus className="size-3" /> | ||
| </Button> | ||
| </TooltipTrigger> | ||
| <TooltipContent side="bottom">New File</TooltipContent> | ||
| </Tooltip> | ||
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <Button variant="ghost" size="icon" className="size-5"> | ||
| <FolderPlus className="size-3" /> | ||
| </Button> | ||
| </TooltipTrigger> | ||
| <TooltipContent side="bottom">New Folder</TooltipContent> | ||
| </Tooltip> | ||
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <Button | ||
| variant="ghost" | ||
| size="icon" | ||
| className="size-5" | ||
| onClick={() => void handleRefresh()} | ||
| > | ||
| <RefreshCw className="size-3" /> | ||
| </Button> | ||
| </TooltipTrigger> | ||
| <TooltipContent side="bottom">Refresh</TooltipContent> | ||
| </Tooltip> | ||
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <Button | ||
| variant="ghost" | ||
| size="icon" | ||
| className="size-5" | ||
| onClick={fileTree.collapseAll} | ||
| > | ||
| <Shrink className="size-3" /> | ||
| </Button> | ||
| </TooltipTrigger> | ||
| <TooltipContent side="bottom">Collapse All</TooltipContent> |
Contributor
There was a problem hiding this comment.
Make the explorer toolbar usable without a mouse.
At Lines 199-240, the action strip is only revealed by group-hover, so keyboard users can tab onto invisible controls. The icon-only buttons also have no accessible name, and the New File / New Folder buttons are enabled despite having no handler. Show the strip on group-focus-within, add aria-labels, and hide or disable unfinished buttons until they're wired.
♿ Proposed hardening
- <div className="flex items-center gap-0.5 opacity-0 group-hover:opacity-100 transition-opacity">
+ <div className="flex items-center gap-0.5 opacity-0 transition-opacity group-hover:opacity-100 group-focus-within:opacity-100">
<Tooltip>
<TooltipTrigger asChild>
- <Button variant="ghost" size="icon" className="size-5">
+ <Button aria-label="New File" variant="ghost" size="icon" className="size-5">
<FilePlus className="size-3" />
</Button>
</TooltipTrigger>
@@
<Tooltip>
<TooltipTrigger asChild>
- <Button variant="ghost" size="icon" className="size-5">
+ <Button aria-label="New Folder" variant="ghost" size="icon" className="size-5">
<FolderPlus className="size-3" />
</Button>
</TooltipTrigger>
@@
<Button
+ aria-label="Refresh"
variant="ghost"
size="icon"
className="size-5"
@@
<Button
+ aria-label="Collapse All"
variant="ghost"
size="icon"
className="size-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
| <div className="flex items-center gap-0.5 opacity-0 group-hover:opacity-100 transition-opacity"> | |
| <Tooltip> | |
| <TooltipTrigger asChild> | |
| <Button variant="ghost" size="icon" className="size-5"> | |
| <FilePlus className="size-3" /> | |
| </Button> | |
| </TooltipTrigger> | |
| <TooltipContent side="bottom">New File</TooltipContent> | |
| </Tooltip> | |
| <Tooltip> | |
| <TooltipTrigger asChild> | |
| <Button variant="ghost" size="icon" className="size-5"> | |
| <FolderPlus className="size-3" /> | |
| </Button> | |
| </TooltipTrigger> | |
| <TooltipContent side="bottom">New Folder</TooltipContent> | |
| </Tooltip> | |
| <Tooltip> | |
| <TooltipTrigger asChild> | |
| <Button | |
| variant="ghost" | |
| size="icon" | |
| className="size-5" | |
| onClick={() => void handleRefresh()} | |
| > | |
| <RefreshCw className="size-3" /> | |
| </Button> | |
| </TooltipTrigger> | |
| <TooltipContent side="bottom">Refresh</TooltipContent> | |
| </Tooltip> | |
| <Tooltip> | |
| <TooltipTrigger asChild> | |
| <Button | |
| variant="ghost" | |
| size="icon" | |
| className="size-5" | |
| onClick={fileTree.collapseAll} | |
| > | |
| <Shrink className="size-3" /> | |
| </Button> | |
| </TooltipTrigger> | |
| <TooltipContent side="bottom">Collapse All</TooltipContent> | |
| <div className="flex items-center gap-0.5 opacity-0 transition-opacity group-hover:opacity-100 group-focus-within:opacity-100"> | |
| <Tooltip> | |
| <TooltipTrigger asChild> | |
| <Button aria-label="New File" variant="ghost" size="icon" className="size-5"> | |
| <FilePlus className="size-3" /> | |
| </Button> | |
| </TooltipTrigger> | |
| <TooltipContent side="bottom">New File</TooltipContent> | |
| </Tooltip> | |
| <Tooltip> | |
| <TooltipTrigger asChild> | |
| <Button aria-label="New Folder" variant="ghost" size="icon" className="size-5"> | |
| <FolderPlus className="size-3" /> | |
| </Button> | |
| </TooltipTrigger> | |
| <TooltipContent side="bottom">New Folder</TooltipContent> | |
| </Tooltip> | |
| <Tooltip> | |
| <TooltipTrigger asChild> | |
| <Button | |
| aria-label="Refresh" | |
| variant="ghost" | |
| size="icon" | |
| className="size-5" | |
| onClick={() => void handleRefresh()} | |
| > | |
| <RefreshCw className="size-3" /> | |
| </Button> | |
| </TooltipTrigger> | |
| <TooltipContent side="bottom">Refresh</TooltipContent> | |
| </Tooltip> | |
| <Tooltip> | |
| <TooltipTrigger asChild> | |
| <Button | |
| aria-label="Collapse All" | |
| variant="ghost" | |
| size="icon" | |
| className="size-5" | |
| onClick={fileTree.collapseAll} | |
| > | |
| <Shrink className="size-3" /> | |
| </Button> | |
| </TooltipTrigger> | |
| <TooltipContent side="bottom">Collapse All</TooltipContent> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/RightSidebar/components/FilesTab/FilesTab.tsx
around lines 199 - 240, The explorer toolbar is only revealed on mouse hover and
its icon-only Buttons lack accessible names and have active but unimplemented
actions; update the toolbar container in FilesTab.tsx to also reveal on keyboard
focus by adding the group-focus-within:opacity-100 (alongside
group-hover:opacity-100) so keyboard users can tab into it, add explicit
aria-label attributes to each icon Button (e.g., Buttons wrapped by
Tooltip/TooltipTrigger for New File, New Folder, Refresh, Collapse All) so
screen readers announce them, and mark the unfinished actions (the New File and
New Folder Buttons) as disabled (or remove their tabIndex/put aria-hidden) until
their handlers are implemented; keep the existing handlers like handleRefresh
and fileTree.collapseAll as interactive and accessible.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
5 issues found across 19 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/FilesTab.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/FilesTab.tsx:141">
P1: Handle rejections from `fileTree.reveal(...)` to avoid unhandled promise rejections in the renderer.
(Based on your team's feedback about handling async rejections with explicit catch/logging.) [FEEDBACK_USED]</violation>
<violation number="2" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/FilesTab.tsx:222">
P1: Do not discard the refresh promise without rejection handling; attach `.catch(...)` (or handle errors inside `handleRefresh`) to prevent unhandled promise rejections.
(Based on your team's feedback about handling async rejections with explicit catch/logging.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsx:57">
P2: `zIndex` decreases with depth without a lower bound, so deep directories can get non-positive stacking order and their sticky headers may render behind rows.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/FolderContextMenu/FolderContextMenu.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/FolderContextMenu/FolderContextMenu.tsx:18">
P3: This component duplicates a large shared menu block from `FileContextMenu`; extract the common actions into a shared menu section/helper to prevent behavior and shortcut drift.
(Based on your team's feedback about avoiding duplicating business logic in multiple components.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/FileContextMenu/FileContextMenu.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/FileContextMenu/FileContextMenu.tsx:10">
P2: All context menu items ("Reveal in Finder", "Copy Path", "Delete", etc.) render as fully interactive but have no `onClick` handlers. The destructive-styled "Delete" entry is especially misleading — clicking it silently does nothing. Either wire up the straightforward items (Copy Path, Reveal in Finder are simple IPC calls), or add `disabled` to unimplemented items so users aren't misled. Same applies to `FolderContextMenu`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| variant="ghost" | ||
| size="icon" | ||
| className="size-5" | ||
| onClick={() => void handleRefresh()} |
Contributor
There was a problem hiding this comment.
P1: Do not discard the refresh promise without rejection handling; attach .catch(...) (or handle errors inside handleRefresh) to prevent unhandled promise rejections.
(Based on your team's feedback about handling async rejections with explicit catch/logging.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/FilesTab.tsx, line 222:
<comment>Do not discard the refresh promise without rejection handling; attach `.catch(...)` (or handle errors inside `handleRefresh`) to prevent unhandled promise rejections.
(Based on your team's feedback about handling async rejections with explicit catch/logging.) </comment>
<file context>
@@ -0,0 +1,274 @@
+ variant="ghost"
+ size="icon"
+ className="size-5"
+ onClick={() => void handleRefresh()}
+ >
+ <RefreshCw className="size-3" />
</file context>
| selectedFilePath !== prevSelectedRef.current && | ||
| rootPath | ||
| ) { | ||
| void fileTree.reveal(selectedFilePath).then(() => { |
Contributor
There was a problem hiding this comment.
P1: Handle rejections from fileTree.reveal(...) to avoid unhandled promise rejections in the renderer.
(Based on your team's feedback about handling async rejections with explicit catch/logging.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/FilesTab.tsx, line 141:
<comment>Handle rejections from `fileTree.reveal(...)` to avoid unhandled promise rejections in the renderer.
(Based on your team's feedback about handling async rejections with explicit catch/logging.) </comment>
<file context>
@@ -0,0 +1,274 @@
+ selectedFilePath !== prevSelectedRef.current &&
+ rootPath
+ ) {
+ void fileTree.reveal(selectedFilePath).then(() => {
+ requestAnimationFrame(() => {
+ const el = scrollContainerRef.current?.querySelector(
</file context>
| ? { | ||
| position: "sticky" as const, | ||
| top: depth * rowHeight, | ||
| zIndex: 10 - depth, |
Contributor
There was a problem hiding this comment.
P2: zIndex decreases with depth without a lower bound, so deep directories can get non-positive stacking order and their sticky headers may render behind rows.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsx, line 57:
<comment>`zIndex` decreases with depth without a lower bound, so deep directories can get non-positive stacking order and their sticky headers may render behind rows.</comment>
<file context>
@@ -0,0 +1,86 @@
+ ? {
+ position: "sticky" as const,
+ top: depth * rowHeight,
+ zIndex: 10 - depth,
+ }
+ : {}),
</file context>
Suggested change
| zIndex: 10 - depth, | |
| zIndex: Math.max(1, 10 - depth), |
| Reveal in Finder | ||
| <ContextMenuShortcut>⌥⌘R</ContextMenuShortcut> | ||
| </ContextMenuItem> | ||
| <ContextMenuItem>Open in Integrated Terminal</ContextMenuItem> |
Contributor
There was a problem hiding this comment.
P3: This component duplicates a large shared menu block from FileContextMenu; extract the common actions into a shared menu section/helper to prevent behavior and shortcut drift.
(Based on your team's feedback about avoiding duplicating business logic in multiple components.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/FolderContextMenu/FolderContextMenu.tsx, line 18:
<comment>This component duplicates a large shared menu block from `FileContextMenu`; extract the common actions into a shared menu section/helper to prevent behavior and shortcut drift.
(Based on your team's feedback about avoiding duplicating business logic in multiple components.) </comment>
<file context>
@@ -0,0 +1,39 @@
+ Reveal in Finder
+ <ContextMenuShortcut>⌥⌘R</ContextMenuShortcut>
+ </ContextMenuItem>
+ <ContextMenuItem>Open in Integrated Terminal</ContextMenuItem>
+ <ContextMenuSeparator />
+ <ContextMenuItem>
</file context>
Add inline creation for files and folders in the sidebar file tree: - New File / New Folder toolbar buttons resolve target from selection - Inline input renders at correct tree position with dynamic file icon - Supports nested paths (e.g. "src/components/Button.tsx") - Right-click folder context menu triggers creation inside that folder - Enter creates, Escape/blur cancels - File watcher auto-refreshes tree after creation
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/RightSidebar/components/FilesTab/FilesTab.tsx:
- Around line 288-327: handleNewItemSubmit builds paths using raw user input and
swallows errors; sanitize the incoming name (trim whitespace, remove
leading/trailing slashes, collapse consecutive slashes, and reject empty or
purely-relative segments like "."/"" and optionally guard against "..") before
constructing absolutePath/dirPath, then use the sanitizedName when calling
createDirectory.mutateAsync and writeFile.mutateAsync; also replace the empty
catch with proper error handling: log the error and surface a user-facing
toast/notification (and keep calling setCreating(null) after handling) so
failures are visible; reference the handleNewItemSubmit function and the calls
to createDirectory.mutateAsync, writeFile.mutateAsync, setCreating, and
onSelectFile when making changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 955c78cf-4b67-462a-a954-ed9550f10ea8
📒 Files selected for processing (5)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/FilesTab.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/components/NewItemInput/NewItemInput.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/components/NewItemInput/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/FolderContextMenu/FolderContextMenu.tsx
✅ Files skipped from review due to trivial changes (2)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/components/NewItemInput/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/FolderContextMenu/FolderContextMenu.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsx
Comment on lines
+288
to
+327
| const handleNewItemSubmit = useCallback( | ||
| async (name: string) => { | ||
| if (!creating || !rootPath) return; | ||
| const { mode, parentPath } = creating; | ||
| const segments = name.split("/").filter(Boolean); | ||
| if (segments.length === 0) return; | ||
|
|
||
| const absolutePath = `${parentPath}/${name}`; | ||
|
|
||
| try { | ||
| if (mode === "folder") { | ||
| await createDirectory.mutateAsync({ | ||
| workspaceId, | ||
| absolutePath, | ||
| recursive: true, | ||
| }); | ||
| } else { | ||
| if (segments.length > 1) { | ||
| const dirPath = `${parentPath}/${segments.slice(0, -1).join("/")}`; | ||
| await createDirectory.mutateAsync({ | ||
| workspaceId, | ||
| absolutePath: dirPath, | ||
| recursive: true, | ||
| }); | ||
| } | ||
| await writeFile.mutateAsync({ | ||
| workspaceId, | ||
| absolutePath, | ||
| content: "", | ||
| options: { create: true, overwrite: false }, | ||
| }); | ||
| onSelectFile(absolutePath); | ||
| } | ||
| } catch { | ||
| // TODO: error toast | ||
| } | ||
| setCreating(null); | ||
| }, | ||
| [creating, rootPath, workspaceId, writeFile, createDirectory, onSelectFile], | ||
| ); |
Contributor
There was a problem hiding this comment.
Path construction doesn't sanitize user input; errors are silently swallowed.
Two issues:
-
Line 295: Uses the raw
namedirectly in path construction. If the user enters/foo.txtorfoo/, the resultingabsolutePathwill have double slashes or trailing slashes, which may cause filesystem inconsistencies. -
Line 321: The empty
catchblock means creation failures are invisible to users—the input just disappears with no feedback.
Proposed fix
const handleNewItemSubmit = useCallback(
async (name: string) => {
if (!creating || !rootPath) return;
const { mode, parentPath } = creating;
const segments = name.split("/").filter(Boolean);
if (segments.length === 0) return;
- const absolutePath = `${parentPath}/${name}`;
+ const sanitizedName = segments.join("/");
+ const absolutePath = `${parentPath}/${sanitizedName}`;
try {
if (mode === "folder") {
// ... existing code
}
- } catch {
- // TODO: error toast
+ } catch (error) {
+ console.error("Failed to create item:", error);
+ // TODO: show error toast to user
}
setCreating(null);
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/RightSidebar/components/FilesTab/FilesTab.tsx
around lines 288 - 327, handleNewItemSubmit builds paths using raw user input
and swallows errors; sanitize the incoming name (trim whitespace, remove
leading/trailing slashes, collapse consecutive slashes, and reject empty or
purely-relative segments like "."/"" and optionally guard against "..") before
constructing absolutePath/dirPath, then use the sanitizedName when calling
createDirectory.mutateAsync and writeFile.mutateAsync; also replace the empty
catch with proper error handling: log the error and surface a user-facing
toast/notification (and keep calling setCreating(null) after handling) so
failures are visible; reference the handleNewItemSubmit function and the calls
to createDirectory.mutateAsync, writeFile.mutateAsync, setCreating, and
onSelectFile when making changes.
Contributor
There was a problem hiding this comment.
3 issues found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/components/NewItemInput/NewItemInput.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/components/NewItemInput/NewItemInput.tsx:30">
P2: Ignore Enter while IME composition is active; otherwise users can accidentally submit while confirming composed text.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/FilesTab.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/FilesTab.tsx:295">
P2: Use `segments.join("/")` instead of raw `name` for path construction. The `segments` array already filters empty parts via `split("/").filter(Boolean)`, but the raw `name` is used for the actual path — so inputs like `/foo.txt` or `foo/` produce paths with double slashes or trailing slashes.</violation>
<violation number="2" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/FilesTab.tsx:321">
P2: Empty `catch` block silently swallows file/directory creation errors. At minimum log a warning so failures are observable in dev tools.
(Based on your team's feedback about avoiding empty catch blocks that hide failures.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| }, []); | ||
|
|
||
| const handleKeyDown = (e: React.KeyboardEvent) => { | ||
| if (e.key === "Enter") { |
Contributor
There was a problem hiding this comment.
P2: Ignore Enter while IME composition is active; otherwise users can accidentally submit while confirming composed text.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/components/NewItemInput/NewItemInput.tsx, line 30:
<comment>Ignore Enter while IME composition is active; otherwise users can accidentally submit while confirming composed text.</comment>
<file context>
@@ -0,0 +1,78 @@
+ }, []);
+
+ const handleKeyDown = (e: React.KeyboardEvent) => {
+ if (e.key === "Enter") {
+ e.preventDefault();
+ const trimmed = value.trim();
</file context>
| const segments = name.split("/").filter(Boolean); | ||
| if (segments.length === 0) return; | ||
|
|
||
| const absolutePath = `${parentPath}/${name}`; |
Contributor
There was a problem hiding this comment.
P2: Use segments.join("/") instead of raw name for path construction. The segments array already filters empty parts via split("/").filter(Boolean), but the raw name is used for the actual path — so inputs like /foo.txt or foo/ produce paths with double slashes or trailing slashes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/FilesTab.tsx, line 295:
<comment>Use `segments.join("/")` instead of raw `name` for path construction. The `segments` array already filters empty parts via `split("/").filter(Boolean)`, but the raw `name` is used for the actual path — so inputs like `/foo.txt` or `foo/` produce paths with double slashes or trailing slashes.</comment>
<file context>
@@ -159,6 +254,82 @@ export function FilesTab({
+ const segments = name.split("/").filter(Boolean);
+ if (segments.length === 0) return;
+
+ const absolutePath = `${parentPath}/${name}`;
+
+ try {
</file context>
Suggested change
| const absolutePath = `${parentPath}/${name}`; | |
| const sanitizedName = segments.join("/"); | |
| const absolutePath = `${parentPath}/${sanitizedName}`; |
Contributor
|
Hey @Kitenite @saddlepaddle — nice work on the v2 sidebar! I opened #3153 with a few feature proposals that would build on top of this:
All three would be toggled via Settings. I've looked into the implementation — the architecture supports it well ( Happy to contribute a PR for any or all of these once this lands. Let me know if it aligns with the direction you have in mind for v2. |
- Inline file/folder creation from toolbar buttons and folder context menu - Inline rename with pre-populated name (extension excluded from selection) - Delete with confirmation alert dialog and toast.promise feedback - Reveal in Finder, Copy Path, Copy Relative Path via electronTrpcClient - Unified InlineEditState for create/rename (mutually exclusive) - Make AlertAction.onClick optional for dismiss-only buttons - Fix tree item right padding
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/FilesTab.tsx (1)
294-297:⚠️ Potential issue | 🟡 MinorFiltered segments aren't used in path construction.
Line 294 splits and filters empty segments, but line 297 constructs the path using raw
name. If the user enters/foo.txtorfoo//bar.txt, the resulting path will have unexpected slashes.Proposed fix
const segments = name.split("/").filter(Boolean); if (segments.length === 0) return; - const absolutePath = `${parentPath}/${name}`; + const sanitizedName = segments.join("/"); + const absolutePath = `${parentPath}/${sanitizedName}`;Apply the same pattern to
dirPathconstruction at line 307.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/RightSidebar/components/FilesTab/FilesTab.tsx around lines 294 - 297, The path construction uses the raw `name` which can contain leading/trailing or duplicate slashes; change uses of `name` when building `absolutePath` and `dirPath` to instead join the already-filtered `segments` (from `const segments = name.split("/").filter(Boolean)`) with "/" and append to `parentPath` so generated paths normalize multiple or leading slashes; update the code that sets `absolutePath` and the later `dirPath` to derive from `segments.join("/")` (or empty string handling) rather than `name`.
🧹 Nitpick comments (4)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/FileContextMenu/FileContextMenu.tsx (1)
34-41: Consider handling clipboard write failures.
navigator.clipboard.writeText()can reject (e.g., due to permissions or secure context issues). Unhandled rejections may cause silent failures.Proposed fix
<ContextMenuItem onSelect={() => { - navigator.clipboard.writeText(absolutePath); - toast.success("Path copied"); + navigator.clipboard.writeText(absolutePath).then( + () => toast.success("Path copied"), + () => toast.error("Failed to copy path"), + ); }} >Apply similarly to the "Copy Relative Path" item.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/RightSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/FileContextMenu/FileContextMenu.tsx around lines 34 - 41, The Copy Path context menu currently calls navigator.clipboard.writeText(absolutePath) without handling promise rejections; update the onSelect handler in the ContextMenuItem for "Copy Path" to await or then/catch the writeText call and show toast.success on success and toast.error (or similar) on failure, and apply the same robust handling to the "Copy Relative Path" ContextMenuItem that uses navigator.clipboard.writeText for the relativePath variable so clipboard permission or secure-context failures are surfaced to the user.apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/FilesTab.tsx (2)
263-270: Magic delay and potential race with DOM rendering.The 200ms
setTimeoutis a timing hack that may fail if rendering takes longer. A more robust approach would use a ref callback oruseLayoutEffectto focus when the input actually mounts.Alternative approach using ref callback
Pass an
autoFocusprop toNewItemInputor use a ref callback:// In NewItemInput component const inputRef = useCallback((node: HTMLInputElement | null) => { node?.focus(); }, []); // Or simply use autoFocus on the input element <input autoFocus ... />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/RightSidebar/components/FilesTab/FilesTab.tsx around lines 263 - 270, The current setTimeout-based focus (using scrollContainerRef.current?.querySelector("[data-new-item-input]") and a 200ms delay) is brittle; replace it with a robust mount-driven focus: either have the NewItemInput component accept an autoFocus prop or expose a ref callback that calls focus when the input actually mounts, and move the scrollIntoView call to run before focusing (useLayoutEffect in the parent or a ref callback inside NewItemInput). Specifically, remove the setTimeout block, ensure you target the same data-new-item-input element (or its inner input) and implement focus logic inside NewItemInput (or via a forwarded ref) so focus happens deterministically once the element mounts.
33-65:TreeNodeis a large inline component that could be memoized.With deep trees, every state change (hover, selection, inline edit) re-renders all
TreeNodeinstances. Consider extracting and wrapping withReact.memoto skip unchanged subtrees.Sketch
const TreeNode = memo(function TreeNode({ ... }: TreeNodeProps) { // existing implementation });Ensure props are stable (callbacks via
useCallback) for memo to be effective.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/RightSidebar/components/FilesTab/FilesTab.tsx around lines 33 - 65, TreeNode is a heavy inline component causing full-tree re-renders; extract it into its own component and wrap the exported component with React.memo (e.g., const TreeNode = memo(function TreeNode(props: TreeNodeProps) { ... })) so unchanged nodes skip renders. Ensure all callback props referenced by TreeNode (onSelectFile, onToggleDirectory, onInlineEditSubmit, onInlineEditCancel, onNewFile, onNewFolder, onRename, onDelete) and any objects (inlineEdit, selectedFilePath, hoveredPath) are stable when passed from the parent (use useCallback/useMemo in the parent) so memoization is effective. Also keep the prop types (FileTreeNode, InlineEditState) intact when moving the component.apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/FolderContextMenu/FolderContextMenu.tsx (1)
35-60: Consider extracting shared menu items to reduce duplication.The "Reveal in Finder", "Copy Path", and "Copy Relative Path" sections are identical to
FileContextMenu. Extracting these into shared components or a helper would reduce maintenance burden.Example extraction
// SharedContextMenuItems.tsx export function RevealInFinderItem({ absolutePath }: { absolutePath: string }) { return ( <ContextMenuItem onSelect={() => electronTrpcClient.external.openInFinder.mutate(absolutePath)} > Reveal in Finder </ContextMenuItem> ); } export function CopyPathItems({ absolutePath, relativePath }: { absolutePath: string; relativePath?: string }) { return ( <> <ContextMenuItem onSelect={() => { /* ... */ }}> Copy Path </ContextMenuItem> {relativePath && ( <ContextMenuItem onSelect={() => { /* ... */ }}> Copy Relative Path </ContextMenuItem> )} </> ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/RightSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/FolderContextMenu/FolderContextMenu.tsx around lines 35 - 60, Duplicate menu items in FolderContextMenu and FileContextMenu should be extracted into shared components: create a new SharedContextMenuItems (or similar) file exporting e.g. RevealInFinderItem({absolutePath}), CopyPathItem({absolutePath}), and CopyRelativePathItem({relativePath}) (or a combined CopyPathItems({absolutePath, relativePath})); each should call electronTrpcClient.external.openInFinder.mutate(absolutePath), navigator.clipboard.writeText(...), and toast.success(...) respectively. Replace the duplicated JSX in FolderContextMenu and FileContextMenu with these shared components, passing absolutePath and relativePath as props so behavior stays identical but duplication is removed.
🤖 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/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/RightSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/FileContextMenu/FileContextMenu.tsx:
- Line 24: The "Open to the Side" ContextMenuItem in FileContextMenu.tsx is a
stub with no click handler; either wire it to an action or disable/hide it. Add
an onSelect handler on the ContextMenuItem (e.g., call a new or existing
function like openFileInSidePane or openToSide passing the file id/entry) to
perform the open-to-side behavior and close the menu, or if the feature is not
ready set the ContextMenuItem to disabled (isDisabled or disabled prop) or
remove it from the menu until implemented so clicks do nothing silently. Ensure
you update any calling code in FileContextMenu (where ContextMenuItem is
rendered) to reference the chosen handler name.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/RightSidebar/components/FilesTab/FilesTab.tsx:
- Around line 362-394: The alert's onClick currently calls
toast.promise(deletePath.mutateAsync(...)) but doesn't return/await the promise,
causing the confirmation dialog to close before the mutation finishes; update
the handleDelete (the useCallback) so the onClick is async and returns the
toast.promise (or awaits it) — specifically change the onClick inside alert
actions to either "onClick: async () => await
toast.promise(deletePath.mutateAsync({ workspaceId, absolutePath }), {...})" or
"onClick: () => toast.promise(...)" with a returned promise, ensuring
deletePath.mutateAsync and toast.promise are awaited/returned so the dialog
stays open until the deletion completes and errors are handled.
---
Duplicate comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/RightSidebar/components/FilesTab/FilesTab.tsx:
- Around line 294-297: The path construction uses the raw `name` which can
contain leading/trailing or duplicate slashes; change uses of `name` when
building `absolutePath` and `dirPath` to instead join the already-filtered
`segments` (from `const segments = name.split("/").filter(Boolean)`) with "/"
and append to `parentPath` so generated paths normalize multiple or leading
slashes; update the code that sets `absolutePath` and the later `dirPath` to
derive from `segments.join("/")` (or empty string handling) rather than `name`.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/RightSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/FileContextMenu/FileContextMenu.tsx:
- Around line 34-41: The Copy Path context menu currently calls
navigator.clipboard.writeText(absolutePath) without handling promise rejections;
update the onSelect handler in the ContextMenuItem for "Copy Path" to await or
then/catch the writeText call and show toast.success on success and toast.error
(or similar) on failure, and apply the same robust handling to the "Copy
Relative Path" ContextMenuItem that uses navigator.clipboard.writeText for the
relativePath variable so clipboard permission or secure-context failures are
surfaced to the user.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/RightSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/FolderContextMenu/FolderContextMenu.tsx:
- Around line 35-60: Duplicate menu items in FolderContextMenu and
FileContextMenu should be extracted into shared components: create a new
SharedContextMenuItems (or similar) file exporting e.g.
RevealInFinderItem({absolutePath}), CopyPathItem({absolutePath}), and
CopyRelativePathItem({relativePath}) (or a combined CopyPathItems({absolutePath,
relativePath})); each should call
electronTrpcClient.external.openInFinder.mutate(absolutePath),
navigator.clipboard.writeText(...), and toast.success(...) respectively. Replace
the duplicated JSX in FolderContextMenu and FileContextMenu with these shared
components, passing absolutePath and relativePath as props so behavior stays
identical but duplication is removed.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/RightSidebar/components/FilesTab/FilesTab.tsx:
- Around line 263-270: The current setTimeout-based focus (using
scrollContainerRef.current?.querySelector("[data-new-item-input]") and a 200ms
delay) is brittle; replace it with a robust mount-driven focus: either have the
NewItemInput component accept an autoFocus prop or expose a ref callback that
calls focus when the input actually mounts, and move the scrollIntoView call to
run before focusing (useLayoutEffect in the parent or a ref callback inside
NewItemInput). Specifically, remove the setTimeout block, ensure you target the
same data-new-item-input element (or its inner input) and implement focus logic
inside NewItemInput (or via a forwarded ref) so focus happens deterministically
once the element mounts.
- Around line 33-65: TreeNode is a heavy inline component causing full-tree
re-renders; extract it into its own component and wrap the exported component
with React.memo (e.g., const TreeNode = memo(function TreeNode(props:
TreeNodeProps) { ... })) so unchanged nodes skip renders. Ensure all callback
props referenced by TreeNode (onSelectFile, onToggleDirectory,
onInlineEditSubmit, onInlineEditCancel, onNewFile, onNewFolder, onRename,
onDelete) and any objects (inlineEdit, selectedFilePath, hoveredPath) are stable
when passed from the parent (use useCallback/useMemo in the parent) so
memoization is effective. Also keep the prop types (FileTreeNode,
InlineEditState) intact when moving the component.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6fb002b1-eb14-4675-b081-b0c966055200
📒 Files selected for processing (6)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/FilesTab.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/components/NewItemInput/NewItemInput.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/FileContextMenu/FileContextMenu.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/FolderContextMenu/FolderContextMenu.tsxpackages/ui/src/atoms/Alert/Alert.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/components/NewItemInput/NewItemInput.tsx
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsx
| }: FileContextMenuProps) { | ||
| return ( | ||
| <ContextMenuContent className="w-56"> | ||
| <ContextMenuItem>Open to the Side</ContextMenuItem> |
Contributor
There was a problem hiding this comment.
Stub menu item does nothing on click.
"Open to the Side" has no onSelect handler, so clicking it silently closes the menu without any action. Either wire it to a handler or hide/disable it until implemented.
Proposed fix options
Option 1: Hide until implemented
- <ContextMenuItem>Open to the Side</ContextMenuItem>
- <ContextMenuSeparator />
+ {/* TODO: Wire to split-view handler */}Option 2: Disable with visual indicator
- <ContextMenuItem>Open to the Side</ContextMenuItem>
+ <ContextMenuItem disabled>Open to the Side</ContextMenuItem>📝 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
| <ContextMenuItem>Open to the Side</ContextMenuItem> | |
| <ContextMenuItem disabled>Open to the Side</ContextMenuItem> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/RightSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/FileContextMenu/FileContextMenu.tsx
at line 24, The "Open to the Side" ContextMenuItem in FileContextMenu.tsx is a
stub with no click handler; either wire it to an action or disable/hide it. Add
an onSelect handler on the ContextMenuItem (e.g., call a new or existing
function like openFileInSidePane or openToSide passing the file id/entry) to
perform the open-to-side behavior and close the menu, or if the feature is not
ready set the ContextMenuItem to disabled (isDisabled or disabled prop) or
remove it from the menu until implemented so clicks do nothing silently. Ensure
you update any calling code in FileContextMenu (where ContextMenuItem is
rendered) to reference the chosen handler name.
Comment on lines
+362
to
+394
| const handleDelete = useCallback( | ||
| (absolutePath: string, name: string, isDirectory: boolean) => { | ||
| const itemType = isDirectory ? "folder" : "file"; | ||
| alert({ | ||
| title: `Delete ${name}?`, | ||
| description: `Are you sure you want to delete this ${itemType}? This action cannot be undone.`, | ||
| actions: [ | ||
| { | ||
| label: "Delete", | ||
| variant: "destructive", | ||
| onClick: () => { | ||
| toast.promise( | ||
| deletePath.mutateAsync({ | ||
| workspaceId, | ||
| absolutePath, | ||
| }), | ||
| { | ||
| loading: `Deleting ${name}...`, | ||
| success: `Deleted ${name}`, | ||
| error: `Failed to delete ${name}`, | ||
| }, | ||
| ); | ||
| }, | ||
| }, | ||
| { | ||
| label: "Cancel", | ||
| variant: "ghost", | ||
| }, | ||
| ], | ||
| }); | ||
| }, | ||
| [workspaceId, deletePath], | ||
| ); |
Contributor
There was a problem hiding this comment.
Delete confirmation returns before mutation completes.
The onClick handler fires toast.promise() but doesn't await it or return the promise. The alert dialog closes immediately while the deletion is still in progress. If deletePath fails, the error toast appears but the UI state may be inconsistent (user might try to interact with the "deleted" item).
Proposed fix to await the deletion
{
label: "Delete",
variant: "destructive",
- onClick: () => {
- toast.promise(
+ onClick: async () => {
+ await toast.promise(
deletePath.mutateAsync({
workspaceId,
absolutePath,
}),
{
loading: `Deleting ${name}...`,
success: `Deleted ${name}`,
error: `Failed to delete ${name}`,
},
);
},
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/RightSidebar/components/FilesTab/FilesTab.tsx
around lines 362 - 394, The alert's onClick currently calls
toast.promise(deletePath.mutateAsync(...)) but doesn't return/await the promise,
causing the confirmation dialog to close before the mutation finishes; update
the handleDelete (the useCallback) so the onClick is async and returns the
toast.promise (or awaits it) — specifically change the onClick inside alert
actions to either "onClick: async () => await
toast.promise(deletePath.mutateAsync({ workspaceId, absolutePath }), {...})" or
"onClick: () => toast.promise(...)" with a returned promise, ensuring
deletePath.mutateAsync and toast.promise are awaited/returned so the dialog
stays open until the deletion completes and errors are handled.
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Contributor
There was a problem hiding this comment.
3 issues found across 6 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/FilesTab.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/FilesTab.tsx:324">
P2: Use a cross-platform path utility (e.g., `path.dirname`/`path.join` or an existing path helper) when deriving the parent directory for renames. The current `lastIndexOf("/")` logic fails on Windows paths.</violation>
<violation number="2" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/FilesTab.tsx:372">
P2: The `onClick` handler calls `toast.promise(...)` without `await`ing or returning the promise. Since `Alert`'s `handleAction` does `await action.onClick?.()` to hold the dialog open with a loading indicator, the non-async callback causes the dialog to close immediately while the deletion is still in-flight. Make the handler `async` and `await` the `toast.promise` call so the alert stays open until the mutation settles.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/FolderContextMenu/FolderContextMenu.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/FolderContextMenu/FolderContextMenu.tsx:45">
P2: Await or catch clipboard writes before showing success; current code can show false success and trigger unhandled rejections.
(Based on your team's feedback about handling async calls with proper await/catch.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Comment on lines
+372
to
+373
| onClick: () => { | ||
| toast.promise( |
Contributor
There was a problem hiding this comment.
P2: The onClick handler calls toast.promise(...) without awaiting or returning the promise. Since Alert's handleAction does await action.onClick?.() to hold the dialog open with a loading indicator, the non-async callback causes the dialog to close immediately while the deletion is still in-flight. Make the handler async and await the toast.promise call so the alert stays open until the mutation settles.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/FilesTab.tsx, line 372:
<comment>The `onClick` handler calls `toast.promise(...)` without `await`ing or returning the promise. Since `Alert`'s `handleAction` does `await action.onClick?.()` to hold the dialog open with a loading indicator, the non-async callback causes the dialog to close immediately while the deletion is still in-flight. Make the handler `async` and `await` the `toast.promise` call so the alert stays open until the mutation settles.</comment>
<file context>
@@ -275,68 +258,140 @@ export function FilesTab({
+ {
+ label: "Delete",
+ variant: "destructive",
+ onClick: () => {
+ toast.promise(
+ deletePath.mutateAsync({
</file context>
Suggested change
| onClick: () => { | |
| toast.promise( | |
| onClick: async () => { | |
| await toast.promise( |
Comment on lines
+45
to
+46
| navigator.clipboard.writeText(absolutePath); | ||
| toast.success("Path copied"); |
Contributor
There was a problem hiding this comment.
P2: Await or catch clipboard writes before showing success; current code can show false success and trigger unhandled rejections.
(Based on your team's feedback about handling async calls with proper await/catch.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/FolderContextMenu/FolderContextMenu.tsx, line 45:
<comment>Await or catch clipboard writes before showing success; current code can show false success and trigger unhandled rejections.
(Based on your team's feedback about handling async calls with proper await/catch.) </comment>
<file context>
@@ -2,45 +2,68 @@ import {
- <ContextMenuItem>
+ <ContextMenuItem
+ onSelect={() => {
+ navigator.clipboard.writeText(absolutePath);
+ toast.success("Path copied");
+ }}
</file context>
Suggested change
| navigator.clipboard.writeText(absolutePath); | |
| toast.success("Path copied"); | |
| void navigator.clipboard.writeText(absolutePath) | |
| .then(() => toast.success("Path copied")) | |
| .catch(() => toast.error("Failed to copy path")); |
| } | ||
| } else { | ||
| const { absolutePath } = inlineEdit; | ||
| const parentDir = absolutePath.slice( |
Contributor
There was a problem hiding this comment.
P2: Use a cross-platform path utility (e.g., path.dirname/path.join or an existing path helper) when deriving the parent directory for renames. The current lastIndexOf("/") logic fails on Windows paths.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/FilesTab/FilesTab.tsx, line 324:
<comment>Use a cross-platform path utility (e.g., `path.dirname`/`path.join` or an existing path helper) when deriving the parent directory for renames. The current `lastIndexOf("/")` logic fails on Windows paths.</comment>
<file context>
@@ -275,68 +258,140 @@ export function FilesTab({
- await writeFile.mutateAsync({
+ } else {
+ const { absolutePath } = inlineEdit;
+ const parentDir = absolutePath.slice(
+ 0,
+ absolutePath.lastIndexOf("/"),
</file context>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx (2)
365-384:⚠️ Potential issue | 🟡 MinorReturn the delete promise from the alert action.
toast.promise(...)is started but ignored, so the confirmation can close immediately while the delete is still running. Returning or awaiting the promise keeps the dialog lifecycle aligned with the mutation.Minimal fix
- onClick: () => { - toast.promise( + onClick: () => + toast.promise( deletePath.mutateAsync({ workspaceId, absolutePath, }), { loading: `Deleting ${name}...`, success: `Deleted ${name}`, error: `Failed to delete ${name}`, }, - ); - }, + ),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx around lines 365 - 384, The alert action starts the delete via toast.promise but doesn't return it, allowing the confirmation dialog to close before the mutation finishes; update the onClick handler in the alert action (the function that calls toast.promise around deletePath.mutateAsync) to return the promise (or await and return it) so the alert lifecycle waits for the toast.promise/deletePath.mutateAsync completion.
191-231:⚠️ Potential issue | 🟠 MajorDon't mark the selection as handled before
reveal()can run.
prevSelectedRefis seeded withselectedFilePathand then updated unconditionally. If the tab mounts with an already-open file, the later render with a resolvedrootPathskipsfileTree.reveal(), so the tree never expands or scrolls to that file.Suggested fix
- const prevSelectedRef = useRef(selectedFilePath); + const prevSelectedRef = useRef<string | undefined>(undefined); @@ useEffect(() => { - if ( - selectedFilePath && - selectedFilePath !== prevSelectedRef.current && - rootPath - ) { + if (!selectedFilePath) { + prevSelectedRef.current = undefined; + return; + } + + if (selectedFilePath !== prevSelectedRef.current && rootPath) { void fileTree.reveal(selectedFilePath).then(() => { + prevSelectedRef.current = selectedFilePath; requestAnimationFrame(() => { scrollContainerRef.current ?.querySelector(`[data-filepath="${CSS.escape(selectedFilePath)}"]`) ?.scrollIntoView({ block: "center" }); }); }); } - prevSelectedRef.current = selectedFilePath; }, [selectedFilePath, rootPath, fileTree]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx around lines 191 - 231, The effect is skipping fileTree.reveal() on mount because prevSelectedRef is initialized to selectedFilePath and then overwritten unconditionally; change the logic so prevSelectedRef.current is not set to selectedFilePath until after you either run fileTree.reveal() (and await/then its completion) or determine no reveal is needed (e.g., when selectedFilePath is falsy or already handled). Concretely, seed prevSelectedRef to null/undefined (not selectedFilePath) and inside the useEffect conditional that calls fileTree.reveal(selectedFilePath) only update prevSelectedRef.current = selectedFilePath in the .then() (or after awaiting) so the reveal and subsequent scrollIntoView always run when rootPath becomes available; keep references to prevSelectedRef, selectedFilePath, rootPath, and fileTree.reveal in mind when applying the change.apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/FileContextMenu/FileContextMenu.tsx (1)
24-25:⚠️ Potential issue | 🟡 MinorDisable the unfinished "Open to the Side" action.
This item still closes the menu without doing anything. Either wire it up or hide/disable it until the split-pane flow exists.
Minimal fix
- <ContextMenuItem>Open to the Side</ContextMenuItem> + <ContextMenuItem disabled>Open to the Side</ContextMenuItem>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/FileContextMenu/FileContextMenu.tsx around lines 24 - 25, The "Open to the Side" context menu item in FileContextMenu is unfinished and should be disabled/hid so it doesn't close the menu or give a dead action; locate the ContextMenuItem with the label "Open to the Side" in the FileContextMenu component and either remove it or mark it disabled (e.g., add disabled={true} and an optional title like "Not implemented") so it is visible but inert; if your UI library requires preventing selection to avoid closing the menu, also add a no-op onSelect handler that calls e.preventDefault() / stops propagation to ensure the menu doesn't close when clicked.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx (1)
14-19: Extract the shared tree helpers out of the legacy sidebar module.This new
FilesTabsurface already depends onWorkspaceView/RightSidebar/FilesView/*, and sibling components in this feature do the same forFileIcon. That couples the v2 sidebar to the implementation it is replacing and makes the eventual cleanup harder.As per coding guidelines, "Co-locate component dependencies (utils, hooks, constants, config, stories) next to the file using them."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx around lines 14 - 19, FilesTab imports ROW_HEIGHT and TREE_INDENT from the legacy WorkspaceView/RightSidebar/FilesView module which couples the new v2 sidebar to the old implementation; extract the shared tree helper constants (ROW_HEIGHT, TREE_INDENT) and any related utilities used by NewItemInput and WorkspaceFilesTreeItem into a colocated module next to FilesTab (e.g., filesTab/treeHelpers or components/FilesTab/utils), update FilesTab to import ROW_HEIGHT and TREE_INDENT from that new module, and ensure sibling components (NewItemInput, WorkspaceFilesTreeItem) also import the helpers from the new colocated module to remove the dependency on WorkspaceView/RightSidebar/FilesView.
🤖 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/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx:
- Around line 439-489: The icon-only action Buttons lack accessible names;
update each Button element that uses startCreating("file"),
startCreating("folder"), handleRefresh(), and fileTree.collapseAll to include an
appropriate aria-label (e.g., aria-label="New File", "New Folder", "Refresh",
"Collapse All") so assistive tech can identify the controls; ensure the
aria-label strings match the TooltipContent text for consistency.
- Around line 322-333: The rename branch currently builds
destinationAbsolutePath by concatenating raw name, allowing slashes to turn a
rename into a move; update the logic in the inline rename handling (where
inlineEdit, absolutePath and movePath.mutateAsync are used) to reject or
sanitize path separators in name before calling movePath.mutateAsync: derive a
safe basename (e.g., newName = last segment of name.split("/") or validate that
name does not contain "/") and use that basename to construct destinationPath,
or surface a validation error to the user if slashes are present, then call
movePath.mutateAsync with workspaceId, sourceAbsolutePath: absolutePath, and
destinationAbsolutePath built from the original parentDir + "/" + sanitized
basename.
- Around line 166-170: The code currently derives rootPath from
workspaceQuery.data and therefore shows the "Workspace worktree not available"
UI while workspaceTrpc.workspace.get.useQuery is still loading; change the logic
in FilesTab (and the similar block at the other location) to first check
workspaceQuery.isLoading (or workspaceQuery.status) and render a loading state
(or nothing) while loading, then after loading check for missing
workspaceQuery.data?.worktreePath to render the unavailable-state. Update the
conditional that uses rootPath so it only computes or treats it as missing after
workspaceQuery.isLoading is false and workspaceQuery.data is defined.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/WorkspaceSidebar.tsx:
- Around line 28-39: The icon-only button inside Tooltip lacks an accessible
name; replace the plain Button used in the TooltipTrigger with the project's
IconButton (or at minimum add an aria-label) so the trigger gets a semantic
label. Update the element inside TooltipTrigger (currently using Button, props
variant="ghost", size="icon", className="size-6", onClick) to use IconButton (or
the Button with aria-label={tooltip}) and pass the tooltip text as the
label/aria-label so assistive tech can read it; keep the Icon child and existing
onClick behavior and leave TooltipContent as-is.
---
Duplicate comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/FileContextMenu/FileContextMenu.tsx:
- Around line 24-25: The "Open to the Side" context menu item in FileContextMenu
is unfinished and should be disabled/hid so it doesn't close the menu or give a
dead action; locate the ContextMenuItem with the label "Open to the Side" in the
FileContextMenu component and either remove it or mark it disabled (e.g., add
disabled={true} and an optional title like "Not implemented") so it is visible
but inert; if your UI library requires preventing selection to avoid closing the
menu, also add a no-op onSelect handler that calls e.preventDefault() / stops
propagation to ensure the menu doesn't close when clicked.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx:
- Around line 365-384: The alert action starts the delete via toast.promise but
doesn't return it, allowing the confirmation dialog to close before the mutation
finishes; update the onClick handler in the alert action (the function that
calls toast.promise around deletePath.mutateAsync) to return the promise (or
await and return it) so the alert lifecycle waits for the
toast.promise/deletePath.mutateAsync completion.
- Around line 191-231: The effect is skipping fileTree.reveal() on mount because
prevSelectedRef is initialized to selectedFilePath and then overwritten
unconditionally; change the logic so prevSelectedRef.current is not set to
selectedFilePath until after you either run fileTree.reveal() (and await/then
its completion) or determine no reveal is needed (e.g., when selectedFilePath is
falsy or already handled). Concretely, seed prevSelectedRef to null/undefined
(not selectedFilePath) and inside the useEffect conditional that calls
fileTree.reveal(selectedFilePath) only update prevSelectedRef.current =
selectedFilePath in the .then() (or after awaiting) so the reveal and subsequent
scrollIntoView always run when rootPath becomes available; keep references to
prevSelectedRef, selectedFilePath, rootPath, and fileTree.reveal in mind when
applying the change.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx:
- Around line 14-19: FilesTab imports ROW_HEIGHT and TREE_INDENT from the legacy
WorkspaceView/RightSidebar/FilesView module which couples the new v2 sidebar to
the old implementation; extract the shared tree helper constants (ROW_HEIGHT,
TREE_INDENT) and any related utilities used by NewItemInput and
WorkspaceFilesTreeItem into a colocated module next to FilesTab (e.g.,
filesTab/treeHelpers or components/FilesTab/utils), update FilesTab to import
ROW_HEIGHT and TREE_INDENT from that new module, and ensure sibling components
(NewItemInput, WorkspaceFilesTreeItem) also import the helpers from the new
colocated module to remove the dependency on
WorkspaceView/RightSidebar/FilesView.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c4484fa8-5165-44bc-9122-bf5fb116937b
📒 Files selected for processing (16)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/WorkspaceSidebar.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/NewItemInput/NewItemInput.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/NewItemInput/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/FileContextMenu/FileContextMenu.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/FileContextMenu/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/FolderContextMenu/FolderContextMenu.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/FolderContextMenu/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/SidebarHeader/SidebarHeader.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/SidebarHeader/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsx
💤 Files with no reviewable changes (1)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/index.ts
✅ Files skipped from review due to trivial changes (5)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/FolderContextMenu/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/SidebarHeader/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/NewItemInput/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsx
Comment on lines
+166
to
+170
| const workspaceQuery = workspaceTrpc.workspace.get.useQuery({ | ||
| id: workspaceId, | ||
| }); | ||
| const rootPath = workspaceQuery.data?.worktreePath ?? ""; | ||
|
|
Contributor
There was a problem hiding this comment.
Don't show the unavailable state while the workspace query is still loading.
On a cold render, workspaceQuery.data is undefined, so this flashes "Workspace worktree not available" before workspace.get resolves. Split the loading and missing-worktree states so the initial render does not show a false error.
One simple split
- if (!workspaceQuery.data?.worktreePath) {
+ if (workspaceQuery.isLoading) {
+ return (
+ <div className="flex h-full items-center justify-center text-sm text-muted-foreground">
+ Loading files...
+ </div>
+ );
+ }
+
+ if (!workspaceQuery.data?.worktreePath) {
return (
<div className="flex h-full items-center justify-center text-sm text-muted-foreground">
Workspace worktree not availableAlso applies to: 396-402
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx
around lines 166 - 170, The code currently derives rootPath from
workspaceQuery.data and therefore shows the "Workspace worktree not available"
UI while workspaceTrpc.workspace.get.useQuery is still loading; change the logic
in FilesTab (and the similar block at the other location) to first check
workspaceQuery.isLoading (or workspaceQuery.status) and render a loading state
(or nothing) while loading, then after loading check for missing
workspaceQuery.data?.worktreePath to render the unavailable-state. Update the
conditional that uses rootPath so it only computes or treats it as missing after
workspaceQuery.isLoading is false and workspaceQuery.data is defined.
Comment on lines
+322
to
+333
| } else { | ||
| const { absolutePath } = inlineEdit; | ||
| const parentDir = absolutePath.slice( | ||
| 0, | ||
| absolutePath.lastIndexOf("/"), | ||
| ); | ||
| const destinationPath = `${parentDir}/${name}`; | ||
| await movePath.mutateAsync({ | ||
| workspaceId, | ||
| sourceAbsolutePath: absolutePath, | ||
| destinationAbsolutePath: destinationPath, | ||
| }); |
Contributor
There was a problem hiding this comment.
Keep rename from turning into a move.
The rename branch concatenates raw name into destinationAbsolutePath, so typing / in the inline rename field silently moves the item into another directory. Nested paths make sense for create here, but they are very surprising behind a Rename... action.
One minimal guard
} else {
+ if (name.includes("/")) {
+ toast.error("Rename cannot include path separators");
+ return;
+ }
+
const { absolutePath } = inlineEdit;
const parentDir = absolutePath.slice(
0,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx
around lines 322 - 333, The rename branch currently builds
destinationAbsolutePath by concatenating raw name, allowing slashes to turn a
rename into a move; update the logic in the inline rename handling (where
inlineEdit, absolutePath and movePath.mutateAsync are used) to reject or
sanitize path separators in name before calling movePath.mutateAsync: derive a
safe basename (e.g., newName = last segment of name.split("/") or validate that
name does not contain "/") and use that basename to construct destinationPath,
or surface a validation error to the user if slashes are present, then call
movePath.mutateAsync with workspaceId, sourceAbsolutePath: absolutePath, and
destinationAbsolutePath built from the original parentDir + "/" + sanitized
basename.
Comment on lines
+439
to
+489
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <Button | ||
| variant="ghost" | ||
| size="icon" | ||
| className="size-5" | ||
| onClick={() => void startCreating("file")} | ||
| > | ||
| <FilePlus className="size-3" /> | ||
| </Button> | ||
| </TooltipTrigger> | ||
| <TooltipContent side="bottom">New File</TooltipContent> | ||
| </Tooltip> | ||
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <Button | ||
| variant="ghost" | ||
| size="icon" | ||
| className="size-5" | ||
| onClick={() => void startCreating("folder")} | ||
| > | ||
| <FolderPlus className="size-3" /> | ||
| </Button> | ||
| </TooltipTrigger> | ||
| <TooltipContent side="bottom">New Folder</TooltipContent> | ||
| </Tooltip> | ||
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <Button | ||
| variant="ghost" | ||
| size="icon" | ||
| className="size-5" | ||
| onClick={() => void handleRefresh()} | ||
| > | ||
| <RefreshCw className="size-3" /> | ||
| </Button> | ||
| </TooltipTrigger> | ||
| <TooltipContent side="bottom">Refresh</TooltipContent> | ||
| </Tooltip> | ||
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <Button | ||
| variant="ghost" | ||
| size="icon" | ||
| className="size-5" | ||
| onClick={fileTree.collapseAll} | ||
| > | ||
| <FoldVertical className="size-3" /> | ||
| </Button> | ||
| </TooltipTrigger> | ||
| <TooltipContent side="bottom">Collapse All</TooltipContent> |
Contributor
There was a problem hiding this comment.
Add accessible names to the icon-only file actions.
These buttons only expose an SVG plus a tooltip, so assistive tech sees unlabeled controls. Please add aria-label for New File, New Folder, Refresh, and Collapse All here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx
around lines 439 - 489, The icon-only action Buttons lack accessible names;
update each Button element that uses startCreating("file"),
startCreating("folder"), handleRefresh(), and fileTree.collapseAll to include an
appropriate aria-label (e.g., aria-label="New File", "New Folder", "Refresh",
"Collapse All") so assistive tech can identify the controls; ensure the
aria-label strings match the TooltipContent text for consistency.
Comment on lines
+28
to
+39
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <Button | ||
| variant="ghost" | ||
| size="icon" | ||
| className="size-6" | ||
| onClick={onClick} | ||
| > | ||
| <Icon className="size-3.5" /> | ||
| </Button> | ||
| </TooltipTrigger> | ||
| <TooltipContent side="bottom">{tooltip}</TooltipContent> |
Contributor
There was a problem hiding this comment.
Label the icon-only search action.
TooltipContent does not give the trigger an accessible name, so assistive tech sees an unlabeled button. Adding the label in IconButton fixes this once for every header action that uses it.
Minimal fix
<Button
variant="ghost"
size="icon"
className="size-6"
+ aria-label={tooltip}
onClick={onClick}
>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/WorkspaceSidebar.tsx
around lines 28 - 39, The icon-only button inside Tooltip lacks an accessible
name; replace the plain Button used in the TooltipTrigger with the project's
IconButton (or at minimum add an aria-label) so the trigger gets a semantic
label. Update the element inside TooltipTrigger (currently using Button, props
variant="ghost", size="icon", className="size-6", onClick) to use IconButton (or
the Button with aria-label={tooltip}) and pass the tooltip text as the
label/aria-label so assistive tech can read it; keep the Icon child and existing
onClick behavior and leave TooltipContent as-is.
MocA-Love
added a commit
to MocA-Love/superset
that referenced
this pull request
Apr 5, 2026
cherry-pick方式で内容を取り込み済みの14コミットをgit履歴上もマージ済みにする。 取り込み済み (cherry-pick / 手動移植): - be22b46 ParadevAI#3125 — スキップ (下記参照) - 88bc7fb ParadevAI#3127 — Revert DA1 ✓ - 92d0ff9 ParadevAI#3054 — DA1 fix ✓ - c48450e ParadevAI#3093 — file viewer pane fix ✓ - fffa8db ParadevAI#3128 — version 1.4.7 ✓ - 589a7c7 ParadevAI#3136 — fuzzy scorer (ハイブリッド方式) ✓ - ceb8c81 ParadevAI#3150 — Electron 40.8.5 ✓ - 8922b94 ParadevAI#3137 — terminalId分離 ✓ - c7508e5 ParadevAI#3152 — GitHub無料化 ✓ - 2b91f11 ParadevAI#3155 — v2 terminal theme ✓ - b8b11af ParadevAI#3154 — TUI dimension fix ✓ - 7599ace ParadevAI#3149 — v2 sidebar file tree (手動統合) ✓ - 4d7c612 ParadevAI#3174 — DnD重複削除 ✓ - 864977d ParadevAI#3157 — Host Service分離 ✓ 意図的にスキップ: - be22b46 ParadevAI#3125 (GitHub polling簡素化) フォーク独自のGitHubSyncService (バックエンド集中ポーリング) と 設計が異なるため不採用。upstreamはフロントエンドhover駆動、フォークは バックエンドキャッシュウォーマー方式。詳細は githubQueryPolicy.ts と github-sync-service.ts のFORK NOTEを参照。 ゴースト・マージ復元 (revert 134cfd5 で消失した内容): - 538f306 ParadevAI#3120 — Patch vuln ✓ - 1588d20 ParadevAI#3108 — terminal lifecycle分離 ✓ - 59426f6 ParadevAI#3122 — file tree + FilePane + Alert refactor (手動統合) ✓ - 10d9a5d ParadevAI#3097 — tiptap line-height ✓ - 337a9ae ParadevAI#3121 — Codex hooks削除 ✓
9 tasks
This branch was previously deployed
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
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
Test plan
Summary by cubic
Adds a tabbed right sidebar to the v2 workspace with a recursive file tree, inline create/rename/delete, and context menu actions. Replaces the old flat file list; the header search button now opens quick open.
New Features
useFileTreefrom@superset/workspace-clientwith sticky folder headers, JS hover tracking, reveal + scroll to selection, and context menus (Reveal in Finder, Copy Path/Relative Path, Rename, Delete with confirm).Refactors
RightSidebartoWorkspaceSidebar; removed old file list, search toolbar, anduseWorkspaceFileSearch.FilesTab,SidebarHeader,WorkspaceFilesTreeItem; unified inline edit state for create/rename.workspaceNameand wireonSearchto quick open; madeAlertactiononClickoptional.Written for commit e5ad30d. Summary will update on new commits.
Summary by CodeRabbit
New Features
Improvements
Coming Soon