File tree because we're just that cool#1112
Conversation
Add tab switcher at the top of the right sidebar to switch between Changes and Files views. The Files view is currently a placeholder.
Implement a VSCode-style file explorer in the Files tab of the right sidebar: - Add filesystem tRPC router with CRUD operations (create, rename, delete, move, copy) - Create FilesView component using react-arborist for virtualized tree rendering - Add file type icons with extension-based coloring - Support search/filter, create file/folder, and context menu actions - Add Zustand store for persisting expanded folders and preferences - Share existing react-dnd DragDropManager to avoid backend conflicts
Kitenite
changed the title
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds a full-featured file explorer: new tRPC filesystem router, FilesView UI and supporting components/hooks, file-explorer state, RightSidebar tabbed UI replacing the old Sidebar, shared file-tree types, and minor cleanup/import fixes. Also adds dependency Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant FilesView as FilesView
participant Hook as useFileTree
participant TRPC as electronTrpc
participant Router as Filesystem Router
participant FS as Node fs/promises
User->>FilesView: mount (worktreePath)
FilesView->>Hook: init(worktreePath)
Hook->>TRPC: filesystem.readDirectory(rootPath)
TRPC->>Router: invoke readDirectory
Router->>FS: readdir/stat calls
FS-->>Router: entries
Router-->>TRPC: DirectoryEntry[]
TRPC-->>Hook: entries
Hook-->>FilesView: treeData (children null)
User->>FilesView: expand folder
FilesView->>Hook: loadChildren(nodePath)
Hook->>TRPC: filesystem.readDirectory(folderPath)
TRPC->>Router: readDirectory
Router->>FS: readdir/stat
FS-->>Router: child entries
Router-->>TRPC: DirectoryEntry[]
TRPC-->>Hook: children
Hook-->>FilesView: update tree (cached children)
User->>FilesView: create file via context menu
FilesView->>useFileTreeActions: createFile(parentPath,name)
useFileTreeActions->>TRPC: filesystem.createFile mutation
TRPC->>Router: createFile
Router->>FS: writeFile
FS-->>Router: success
Router-->>TRPC: filepath
TRPC-->>useFileTreeActions: success
useFileTreeActions->>Hook: refetch()
Hook-->>FilesView: refreshed tree
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In `@apps/desktop/src/lib/trpc/routers/filesystem/index.ts`:
- Around line 52-129: The fs.access() catch blocks in createFile,
createDirectory, and rename currently swallow non-ENOENT errors; change them to
explicitly check error.code === "ENOENT" (treat as non-existent) and for the
"already exists" case detect access success and throw a TRPCError with code
BAD_REQUEST (including the name/path); for any other access errors log the error
and throw a TRPCError with code INTERNAL_SERVER_ERROR. Update the
access-handling logic inside createFile, createDirectory, and rename (where
fs.access is called) to distinguish ENOENT vs other errors and to throw
TRPCError instead of generic Error for consistency.
- Around line 18-49: The current .query handler for reading directories swallows
filesystem errors and returns an empty array; instead, catch the error and throw
a TRPCError from '@trpc/server' with appropriate codes: if error.code ===
'ENOENT' throw new TRPCError({ code: 'NOT_FOUND', message: `Directory not found:
${dirPath}`, cause: error }); if error.code === 'EACCES' or other permission
errors throw new TRPCError({ code: 'FORBIDDEN', message: `Permission denied:
${dirPath}`, cause: error }); for other errors throw a TRPCError({ code:
'INTERNAL_SERVER_ERROR', message: 'Failed to read directory', cause: error });
ensure you import TRPCError and remove the fallback return [] in the
readDirectory/.query handler so failures surface to the client.
- Around line 10-17: The router accepts raw filesystem paths and swallows or
throws generic errors; update readDirectory, exists, stat, createFile,
createDirectory, rename, delete, move, and copy to validate paths and use
TRPCError: import and call secureFs.assertRegisteredWorktree() and
secureFs.assertRealpathInWorktree() at the start of each procedure to prevent
../ or symlink escapes and reject absolute paths, convert silent empty/null
returns in readDirectory/exists/stat into TRPCError with NOT_FOUND or
BAD_REQUEST as appropriate, replace generic Error throws in
createFile/createDirectory/rename with TRPCError (BAD_REQUEST for invalid input,
INTERNAL_SERVER_ERROR for unexpected failures), and when delete/move/copy
collect errors convert those into a TRPCError containing details (use
INTERNAL_SERVER_ERROR or NOT_FOUND per case); ensure TRPCError is used
consistently with clear messages and proper codes across the referenced
functions.
- Around line 247-283: The exists and stat procedures currently swallow
filesystem errors; update exists (the publicProcedure named exists that calls
fs.access and fs.stat) to catch errors, check error.code === 'ENOENT' and return
{ exists: false, ... } only for that case, but rethrow other errors as a
TRPCError with code 'INTERNAL_SERVER_ERROR' and include the original error
message; update stat (the publicProcedure named stat that calls fs.stat) to stop
returning null on failure and instead throw a TRPCError with code 'NOT_FOUND'
(include path and original error in the message or meta) so callers receive
proper error semantics.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/components/FileTreeContextMenu/FileTreeContextMenu.tsx`:
- Around line 40-63: The context menu uses parentPath which currently falls back
to worktree root for files; compute the parent directory from node.path for file
nodes and only fall back to worktreePath when there is no parent. Update the
logic around parentPath (used by onNewFile/onNewFolder) so: if node?.isDirectory
use node.path, else if node?.path compute its directory by removing the trailing
segment (e.g., slice up to last '/' or path separator) without importing Node's
path module, and otherwise use worktreePath; keep targetPath and usePathActions
unchanged.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/components/FileTreeToolbar/FileTreeToolbar.tsx`:
- Around line 40-52: The debounce in handleSearchChange is broken because the
timeoutId is scoped inside the handler and its cleanup is never invoked; change
to store the timer in a ref (e.g., timeoutRef) and, inside handleSearchChange
(used in FileTreeToolbar), clear timeoutRef.current before assigning a new
setTimeout with SEARCH_DEBOUNCE_MS and call onSearchChange when it fires; also
add a useEffect cleanup that clears timeoutRef.current on unmount to avoid stray
callbacks.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/components/NewItemInput/NewItemInput.tsx`:
- Around line 6-12: Remove the unused parentPath prop: delete parentPath from
the NewItemInputProps interface and remove the parentPath parameter from the
NewItemInput function signature (currently destructured as _parentPath). Update
any internal references (there should be none) and adjust callers by removing
parentPath={newItemParentPath} from the FilesView call site so the prop is no
longer passed. Ensure TypeScript compiles and run linting to catch any leftover
references.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/FilesView.tsx`:
- Around line 187-195: handleRename currently uses treeData.find which only
checks top-level nodes so nested items never get found; replace that lookup with
a recursive search (e.g., implement a helper like findNodeById that walks
treeData children) or use an existing index to resolve a node by id anywhere in
the tree, then call rename(node.path, name) as before; update handleRename to
call this recursive helper (referencing handleRename, treeData, findNodeById,
and rename) so nested files/folders are correctly located and renamed.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/hooks/useFileTree.ts`:
- Around line 26-48: The childrenCache needs to be invalidated when worktreePath
or showHiddenFiles change: add a useEffect that clears childrenCache (the
in-module cache used for expanded folder children) whenever worktreePath or
showHiddenFiles updates, and perform that invalidation before you create
trpcUtils to ensure subsequent queries use fresh state; change loadChildren to
accept a params object ({ nodeId, nodePath }) instead of positional args and
update any internal callers to pass that object; and extract the hardcoded
staleTime value (5000) from the electronTrpc.filesystem.readDirectory.useQuery
options into a named constant (e.g., DIRECTORY_STALE_TIME) and use that constant
in the query options.
🧹 Nitpick comments (10)
apps/desktop/src/shared/utils/branch.ts (1)
35-51: Good refactoring to centralize sanitization logic.The change ensures all prefixes are consistently sanitized before returning, which is a solid improvement.
One edge case to consider: if the input is truthy but sanitizes to an empty string (e.g.,
customPrefix = "@@@@"), the function returns""instead ofnull. If callers expect either a valid prefix ornull, you may want to coalesce empty strings tonull:💡 Optional: coalesce empty sanitized result to null
- return prefix ? sanitizeSegment(prefix) : null; + const sanitized = prefix ? sanitizeSegment(prefix) : null; + return sanitized || null;apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/components/NewItemInput/NewItemInput.tsx (1)
50-54: Consider extracting indentation constants for clarity.The inline calculation
level * 16 + 4uses magic numbers. While functional, extracting these to named constants would improve readability.♻️ Optional: Extract constants
+const INDENT_PER_LEVEL_PX = 16; +const BASE_PADDING_PX = 4; + export function NewItemInput({ // ... }) { // ... return ( <div className={cn("flex items-center gap-1 px-1 h-7", "bg-accent rounded-sm")} - style={{ paddingLeft: `${level * 16 + 4}px` }} + style={{ paddingLeft: `${level * INDENT_PER_LEVEL_PX + BASE_PADDING_PX}px` }} >apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/utils/file-icons.ts (1)
183-187: Use a params object forgetFileIconinputs.
This avoids positional mistakes and keeps the API extensible.♻️ Suggested change
-export function getFileIcon( - fileName: string, - isDirectory: boolean, - isOpen = false, -): FileIconConfig { +export function getFileIcon({ + fileName, + isDirectory, + isOpen = false, +}: { + fileName: string; + isDirectory: boolean; + isOpen?: boolean; +}): FileIconConfig {As per coding guidelines, Functions with 2+ parameters should accept a single params object with named properties instead of positional arguments.
apps/desktop/src/renderer/stores/file-explorer.ts (1)
14-26: Use params objects for multi-arg store actions.
This reduces ordering mistakes and makes future extension safer.♻️ Example pattern
- toggleFolder: (worktreePath: string, folderId: string) => void; + toggleFolder: (params: { worktreePath: string; folderId: string }) => void;- toggleFolder: (worktreePath, folderId) => { + toggleFolder: ({ worktreePath, folderId }) => {As per coding guidelines, Functions with 2+ parameters should accept a single params object with named properties instead of positional arguments.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/hooks/useFileTreeActions.ts (1)
99-139: Consider using params objects for functions with 2+ parameters.Per coding guidelines, functions with 2+ parameters should accept a single params object with named properties. This improves readability and makes call sites self-documenting.
♻️ Example refactor for createFile
const createFile = useCallback( - (dirPath: string, fileName: string, content = "") => { - createFileMutation.mutate({ dirPath, fileName, content }); + (params: { dirPath: string; fileName: string; content?: string }) => { + createFileMutation.mutate({ + dirPath: params.dirPath, + fileName: params.fileName, + content: params.content ?? "", + }); }, [createFileMutation], );Apply similar pattern to
createDirectory,rename,deleteItems,moveItems, andcopyItems.As per coding guidelines: "Functions with 2+ parameters should accept a single params object with named properties instead of positional arguments."
apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/index.tsx (1)
96-122: Refactor file-open handlers to accept a params object.
handleFileOpenPaneandhandleFileScrollTotake three positional args; switch to a single object and updateChangesView’sonFileOpensignature accordingly to keep call sites self-describing.♻️ Suggested refactor
-const handleFileOpenPane = useCallback( - (file: ChangedFile, category: ChangeCategory, commitHash?: string) => { +const handleFileOpenPane = useCallback( + ({ + file, + category, + commitHash, + }: { + file: ChangedFile; + category: ChangeCategory; + commitHash?: string; + }) => { if (!workspaceId || !worktreePath) return; addFileViewerPane(workspaceId, { filePath: file.path, diffCategory: category, commitHash, oldPath: file.oldPath, }); invalidateFileContent(file.path); }, [workspaceId, worktreePath, addFileViewerPane, invalidateFileContent], ); -const handleFileScrollTo = useCallback( - (file: ChangedFile, category: ChangeCategory, commitHash?: string) => { - scrollToFile(file, category, commitHash); - }, +const handleFileScrollTo = useCallback( + ({ + file, + category, + commitHash, + }: { + file: ChangedFile; + category: ChangeCategory; + commitHash?: string; + }) => { + scrollToFile(file, category, commitHash); + }, [scrollToFile], );As per coding guidelines: Functions with 2+ parameters should accept a single params object with named properties instead of positional arguments.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/hooks/useFileTree.ts (3)
21-88: Avoid derived state + effect fortreeData.
treeDatais fully derived fromrootNodesandbuildTree, so auseMemois enough and removes an extra state/effect pair (place the newtreeDatadeclaration afterbuildTree).♻️ Suggested refactor
- const [treeData, setTreeData] = useState<FileTreeNode[]>([]); const [childrenCache, setChildrenCache] = useState< Record<string, FileTreeNode[]> >({}); @@ const buildTree = useCallback( (nodes: FileTreeNode[]): FileTreeNode[] => { return nodes.map((node) => { @@ [currentExpandedFolders, childrenCache], ); - useEffect(() => { - setTreeData(buildTree(rootNodes)); - }, [rootNodes, buildTree]); + const treeData = useMemo(() => buildTree(rootNodes), [rootNodes, buildTree]);As per coding guidelines: Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
33-47: Extract the 5sstaleTimeinto a named constant.
This avoids magic numbers and keeps cache tuning centralized.♻️ Suggested refactor
+const ROOT_DIR_STALE_TIME_MS = 5_000; + export function useFileTree({ worktreePath, }: UseFileTreeProps): UseFileTreeReturn { @@ } = electronTrpc.filesystem.readDirectory.useQuery( @@ { enabled: !!worktreePath, - staleTime: 5000, + staleTime: ROOT_DIR_STALE_TIME_MS, }, );Based on learnings: Applies to **/*.{ts,tsx} : Avoid magic numbers by extracting them to named constants at module top
10-16: Use a params object forloadChildren.
The hook exposes a 2‑arg function; refactor toloadChildren({ nodeId, nodePath })and update call sites for clarity.♻️ Suggested refactor
interface UseFileTreeReturn { treeData: FileTreeNode[]; isLoading: boolean; error: Error | null; refetch: () => void; - loadChildren: (nodeId: string, nodePath: string) => Promise<FileTreeNode[]>; + loadChildren: (params: { + nodeId: string; + nodePath: string; + }) => Promise<FileTreeNode[]>; } @@ const loadChildren = useCallback( - async (nodeId: string, nodePath: string): Promise<FileTreeNode[]> => { + async ({ + nodeId, + nodePath, + }: { + nodeId: string; + nodePath: string; + }): Promise<FileTreeNode[]> => { if (!worktreePath) return [];As per coding guidelines: Functions with 2+ parameters should accept a single params object with named properties instead of positional arguments.
Also applies to: 90-116
apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/FilesView.tsx (1)
31-64: Pull the default height andstaleTimeinto named constants.
This avoids magic numbers and keeps tuning centralized.♻️ Suggested refactor
+const DEFAULT_TREE_HEIGHT = 400; +const ROOT_DIR_STALE_TIME_MS = 5_000; + export function FilesView() { @@ - const { ref: containerRef, height: treeHeight = 400 } = useResizeObserver(); + const { ref: containerRef, height: treeHeight = DEFAULT_TREE_HEIGHT } = + useResizeObserver(); @@ { enabled: !!worktreePath, - staleTime: 5000, + staleTime: ROOT_DIR_STALE_TIME_MS, }, );Based on learnings: Applies to **/*.{ts,tsx} : Avoid magic numbers by extracting them to named constants at module top
| readDirectory: publicProcedure | ||
| .input( | ||
| z.object({ | ||
| dirPath: z.string(), | ||
| rootPath: z.string(), | ||
| includeHidden: z.boolean().default(false), | ||
| }), | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
wc -l apps/desktop/src/lib/trpc/routers/filesystem/index.tsRepository: superset-sh/superset
Length of output: 121
🏁 Script executed:
cat -n apps/desktop/src/lib/trpc/routers/filesystem/index.tsRepository: superset-sh/superset
Length of output: 9179
🏁 Script executed:
# Search for path validation utilities in the codebase
find apps/desktop -type f -name "*.ts" -o -name "*.tsx" | head -20 | xargs grep -l "resolvePath\|normalizePath\|validatePath" 2>/dev/null | head -10Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Look for imports and path-related utilities in the filesystem directory
ls -la apps/desktop/src/lib/trpc/routers/filesystem/Repository: superset-sh/superset
Length of output: 235
🏁 Script executed:
# Check if there's a shared utility for path validation
find apps/desktop/src/lib -type f \( -name "*.ts" -o -name "*.tsx" \) | xargs grep -l "path.*boundary\|validatePath\|normalizePath" 2>/dev/nullRepository: superset-sh/superset
Length of output: 127
🏁 Script executed:
# Search for TRPCError usage patterns in this file and similar routers
rg "TRPCError" apps/desktop/src/lib/trpc/routers/ -A 2 -B 1 --max-count=15Repository: superset-sh/superset
Length of output: 8337
🏁 Script executed:
cat -n apps/desktop/src/lib/trpc/routers/changes/security/secure-fs.tsRepository: superset-sh/superset
Length of output: 18362
🏁 Script executed:
# Check how other routers validate paths
rg "path.*resolve\|normalized\|FORBIDDEN" apps/desktop/src/lib/trpc/routers/ --max-count=20Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Check the path-validation module referenced in secure-fs
cat -n apps/desktop/src/lib/trpc/routers/changes/security/path-validation.ts | head -100Repository: superset-sh/superset
Length of output: 3624
🏁 Script executed:
# Verify how filesystem router is being called - check if rootPath is meant to be a worktree path
rg "readDirectory\|filesystem\." apps/desktop/src --max-count=15 -A 2 -B 2Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Check if there's middleware or context that validates rootPath
rg "createFilesystemRouter\|filesystem" apps/desktop/src --max-count=20Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Check the path-validation module more completely to understand resolvePathInWorktree
cat -n apps/desktop/src/lib/trpc/routers/changes/security/path-validation.ts | head -200Repository: superset-sh/superset
Length of output: 6563
🏁 Script executed:
# Verify that filesystem router is indeed exposed without guards
rg "createFilesystemRouter" apps/desktop -B 3 -A 3Repository: superset-sh/superset
Length of output: 1840
🏁 Script executed:
# Check what middleware or context is available for context around these procedures
cat -n apps/desktop/src/lib/trpc/index.ts | head -50Repository: superset-sh/superset
Length of output: 1642
Implement path validation and proper TRPC error handling throughout.
These procedures accept raw filesystem paths without boundary checks. A malicious renderer could traverse outside the allowed root using ../ sequences or symlinks. Additionally, the entire router lacks TRPCError usage and silently swallows errors:
readDirectory(line 48),exists(line 259),stat(line 282) return empty/null values on error instead of throwingcreateFile,createDirectory,renamethrow genericErrorinstead ofTRPCErrordelete,move,copycollect errors in arrays without throwing
Use the existing secureFs utilities from apps/desktop/src/lib/trpc/routers/changes/security/secure-fs.ts which provide assertRegisteredWorktree() and assertRealpathInWorktree() to validate paths against worktree boundaries. Throw TRPCError with appropriate codes: NOT_FOUND for missing files, BAD_REQUEST for invalid input (e.g., absolute paths), INTERNAL_SERVER_ERROR for unexpected failures.
🤖 Prompt for AI Agents
In `@apps/desktop/src/lib/trpc/routers/filesystem/index.ts` around lines 10 - 17,
The router accepts raw filesystem paths and swallows or throws generic errors;
update readDirectory, exists, stat, createFile, createDirectory, rename, delete,
move, and copy to validate paths and use TRPCError: import and call
secureFs.assertRegisteredWorktree() and secureFs.assertRealpathInWorktree() at
the start of each procedure to prevent ../ or symlink escapes and reject
absolute paths, convert silent empty/null returns in readDirectory/exists/stat
into TRPCError with NOT_FOUND or BAD_REQUEST as appropriate, replace generic
Error throws in createFile/createDirectory/rename with TRPCError (BAD_REQUEST
for invalid input, INTERNAL_SERVER_ERROR for unexpected failures), and when
delete/move/copy collect errors convert those into a TRPCError containing
details (use INTERNAL_SERVER_ERROR or NOT_FOUND per case); ensure TRPCError is
used consistently with clear messages and proper codes across the referenced
functions.
| .query(async ({ input }): Promise<DirectoryEntry[]> => { | ||
| const { dirPath, rootPath, includeHidden } = input; | ||
|
|
||
| try { | ||
| const entries = await fs.readdir(dirPath, { withFileTypes: true }); | ||
|
|
||
| return entries | ||
| .filter((entry) => includeHidden || !entry.name.startsWith(".")) | ||
| .map((entry) => { | ||
| const fullPath = path.join(dirPath, entry.name); | ||
| const relativePath = path.relative(rootPath, fullPath); | ||
| return { | ||
| id: relativePath, | ||
| name: entry.name, | ||
| path: fullPath, | ||
| relativePath, | ||
| isDirectory: entry.isDirectory(), | ||
| }; | ||
| }) | ||
| .sort((a, b) => { | ||
| if (a.isDirectory !== b.isDirectory) { | ||
| return a.isDirectory ? -1 : 1; | ||
| } | ||
| return a.name.localeCompare(b.name); | ||
| }); | ||
| } catch (error) { | ||
| console.error("[filesystem/readDirectory] Failed:", { | ||
| dirPath, | ||
| error, | ||
| }); | ||
| return []; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, verify the file exists and check its content
ls -la apps/desktop/src/lib/trpc/routers/filesystem/index.tsRepository: superset-sh/superset
Length of output: 166
🏁 Script executed:
# Check the full context of the readDirectory function
head -60 apps/desktop/src/lib/trpc/routers/filesystem/index.tsRepository: superset-sh/superset
Length of output: 1661
🏁 Script executed:
# Search for other error handling patterns in tRPC routes to ensure consistency
rg "throw new TRPCError" apps/desktop/src/lib/trpc/ -A 2 -B 2Repository: superset-sh/superset
Length of output: 5451
🏁 Script executed:
# Check if there are other filesystem operations that handle permissions
cat -n apps/desktop/src/lib/trpc/routers/filesystem/index.ts | tail -100Repository: superset-sh/superset
Length of output: 3226
🏁 Script executed:
# Look for error code handling patterns in the codebase
rg "error.code|error instanceof" apps/desktop/src/lib/trpc/routers/ -B 2 -A 2 | head -50Repository: superset-sh/superset
Length of output: 4401
Don't swallow readDirectory errors by returning an empty list.
Failing reads should surface as TRPCError instead. Use NOT_FOUND for missing directories and FORBIDDEN for permission errors, matching the error context.
🔧 Suggested approach
+import { TRPCError } from "@trpc/server";
...
} catch (error) {
console.error("[filesystem/readDirectory] Failed:", { dirPath, error });
+ const errorMessage = error instanceof Error ? error.message : String(error);
+ if (errorMessage.includes("EACCES")) {
+ throw new TRPCError({
+ code: "FORBIDDEN",
+ message: `Permission denied: ${dirPath}`,
+ });
+ }
+ throw new TRPCError({
+ code: "NOT_FOUND",
+ message: `Directory not found or inaccessible: ${dirPath}`,
+ });
- return [];
}Per coding guidelines: never swallow errors silently. Per tRPC guidelines: use TRPCError with NOT_FOUND when a resource doesn't exist and FORBIDDEN when lacking permission.
🤖 Prompt for AI Agents
In `@apps/desktop/src/lib/trpc/routers/filesystem/index.ts` around lines 18 - 49,
The current .query handler for reading directories swallows filesystem errors
and returns an empty array; instead, catch the error and throw a TRPCError from
'@trpc/server' with appropriate codes: if error.code === 'ENOENT' throw new
TRPCError({ code: 'NOT_FOUND', message: `Directory not found: ${dirPath}`,
cause: error }); if error.code === 'EACCES' or other permission errors throw new
TRPCError({ code: 'FORBIDDEN', message: `Permission denied: ${dirPath}`, cause:
error }); for other errors throw a TRPCError({ code: 'INTERNAL_SERVER_ERROR',
message: 'Failed to read directory', cause: error }); ensure you import
TRPCError and remove the fallback return [] in the readDirectory/.query handler
so failures surface to the client.
| createFile: publicProcedure | ||
| .input( | ||
| z.object({ | ||
| dirPath: z.string(), | ||
| fileName: z.string(), | ||
| content: z.string().default(""), | ||
| }), | ||
| ) | ||
| .mutation(async ({ input }) => { | ||
| const filePath = path.join(input.dirPath, input.fileName); | ||
|
|
||
| try { | ||
| await fs.access(filePath); | ||
| throw new Error(`File already exists: ${input.fileName}`); | ||
| } catch (error) { | ||
| if ( | ||
| error instanceof Error && | ||
| error.message.includes("already exists") | ||
| ) { | ||
| throw error; | ||
| } | ||
| } | ||
|
|
||
| await fs.writeFile(filePath, input.content, "utf-8"); | ||
| return { path: filePath }; | ||
| }), | ||
|
|
||
| createDirectory: publicProcedure | ||
| .input( | ||
| z.object({ | ||
| parentPath: z.string(), | ||
| dirName: z.string(), | ||
| }), | ||
| ) | ||
| .mutation(async ({ input }) => { | ||
| const dirPath = path.join(input.parentPath, input.dirName); | ||
|
|
||
| try { | ||
| await fs.access(dirPath); | ||
| throw new Error(`Directory already exists: ${input.dirName}`); | ||
| } catch (error) { | ||
| if ( | ||
| error instanceof Error && | ||
| error.message.includes("already exists") | ||
| ) { | ||
| throw error; | ||
| } | ||
| } | ||
|
|
||
| await fs.mkdir(dirPath, { recursive: true }); | ||
| return { path: dirPath }; | ||
| }), | ||
|
|
||
| rename: publicProcedure | ||
| .input( | ||
| z.object({ | ||
| oldPath: z.string(), | ||
| newName: z.string(), | ||
| }), | ||
| ) | ||
| .mutation(async ({ input }) => { | ||
| const newPath = path.join(path.dirname(input.oldPath), input.newName); | ||
|
|
||
| try { | ||
| await fs.access(newPath); | ||
| throw new Error(`Target already exists: ${input.newName}`); | ||
| } catch (error) { | ||
| if ( | ||
| error instanceof Error && | ||
| error.message.includes("already exists") | ||
| ) { | ||
| throw error; | ||
| } | ||
| } | ||
|
|
||
| await fs.rename(input.oldPath, newPath); | ||
| return { oldPath: input.oldPath, newPath }; | ||
| }), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Confirm file path and check for other similar patterns
head -5 apps/desktop/src/lib/trpc/routers/filesystem/index.ts
wc -l apps/desktop/src/lib/trpc/routers/filesystem/index.ts
grep -n "fs.access" apps/desktop/src/lib/trpc/routers/filesystem/index.ts
grep -n "TRPCError" apps/desktop/src/lib/trpc/routers/filesystem/index.tsRepository: superset-sh/superset
Length of output: 526
🏁 Script executed:
# Check what fs module is being used and if TRPCError is imported
head -30 apps/desktop/src/lib/trpc/routers/filesystem/index.tsRepository: superset-sh/superset
Length of output: 1012
🏁 Script executed:
# Look for similar catch patterns with message checking
rg -A 5 "error.message.includes" apps/desktop/src/lib/trpc/routers/filesystem/index.tsRepository: superset-sh/superset
Length of output: 336
🏁 Script executed:
# Check the other fs.access calls at lines 178, 221, 251 to see if same pattern repeats
sed -n '170,230p' apps/desktop/src/lib/trpc/routers/filesystem/index.ts | cat -n
sed -n '245,260p' apps/desktop/src/lib/trpc/routers/filesystem/index.ts | cat -nRepository: superset-sh/superset
Length of output: 2646
Fix error handling in fs.access() calls to prevent masking permission and I/O errors.
The fs.access() pattern in createFile, createDirectory, and rename silently swallows non-ENOENT errors (permission denied, I/O errors, etc.) by filtering catch blocks on message content. This masks real failures and violates error handling guidelines.
Update all three procedures to:
- Explicitly handle ENOENT (expected—file/directory doesn't exist)
- Throw
TRPCErrorwith codeBAD_REQUESTwhen target already exists - Log and throw
TRPCErrorwith codeINTERNAL_SERVER_ERRORfor unexpected access failures
Example fix
+import { TRPCError } from "@trpc/server";
...
try {
await fs.access(filePath);
- throw new Error(`File already exists: ${input.fileName}`);
+ throw new TRPCError({
+ code: "BAD_REQUEST",
+ message: `File already exists: ${input.fileName}`,
+ });
} catch (error) {
- if (error instanceof Error && error.message.includes("already exists")) {
- throw error;
- }
+ if (error instanceof TRPCError) throw error;
+ const err = error as NodeJS.ErrnoException;
+ if (err?.code !== "ENOENT") {
+ console.error("[filesystem/createFile] access failed:", { filePath, error });
+ throw new TRPCError({ code: "INTERNAL_SERVER_ERROR" });
+ }
}🤖 Prompt for AI Agents
In `@apps/desktop/src/lib/trpc/routers/filesystem/index.ts` around lines 52 - 129,
The fs.access() catch blocks in createFile, createDirectory, and rename
currently swallow non-ENOENT errors; change them to explicitly check error.code
=== "ENOENT" (treat as non-existent) and for the "already exists" case detect
access success and throw a TRPCError with code BAD_REQUEST (including the
name/path); for any other access errors log the error and throw a TRPCError with
code INTERNAL_SERVER_ERROR. Update the access-handling logic inside createFile,
createDirectory, and rename (where fs.access is called) to distinguish ENOENT vs
other errors and to throw TRPCError instead of generic Error for consistency.
| exists: publicProcedure | ||
| .input(z.object({ path: z.string() })) | ||
| .query(async ({ input }) => { | ||
| try { | ||
| await fs.access(input.path); | ||
| const stats = await fs.stat(input.path); | ||
| return { | ||
| exists: true, | ||
| isDirectory: stats.isDirectory(), | ||
| isFile: stats.isFile(), | ||
| }; | ||
| } catch { | ||
| return { exists: false, isDirectory: false, isFile: false }; | ||
| } | ||
| }), | ||
|
|
||
| stat: publicProcedure | ||
| .input(z.object({ path: z.string() })) | ||
| .query(async ({ input }) => { | ||
| try { | ||
| const stats = await fs.stat(input.path); | ||
| return { | ||
| size: stats.size, | ||
| isDirectory: stats.isDirectory(), | ||
| isFile: stats.isFile(), | ||
| isSymbolicLink: stats.isSymbolicLink(), | ||
| createdAt: stats.birthtime.toISOString(), | ||
| modifiedAt: stats.mtime.toISOString(), | ||
| accessedAt: stats.atime.toISOString(), | ||
| }; | ||
| } catch (error) { | ||
| console.error("[filesystem/stat] Failed:", { | ||
| path: input.path, | ||
| error, | ||
| }); | ||
| return null; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/desktop/src/lib/trpc/routers/filesystem/index.ts | sed -n '240,290p'Repository: superset-sh/superset
Length of output: 1464
🏁 Script executed:
head -40 apps/desktop/src/lib/trpc/routers/filesystem/index.tsRepository: superset-sh/superset
Length of output: 1263
🏁 Script executed:
grep -n "TRPCError" apps/desktop/src/lib/trpc/routers/filesystem/index.tsRepository: superset-sh/superset
Length of output: 46
🏁 Script executed:
find apps/desktop/src/lib/trpc/routers -name "*.ts" -o -name "*.tsx" | head -5Repository: superset-sh/superset
Length of output: 345
🏁 Script executed:
grep -l "TRPCError" apps/desktop/src/lib/trpc/routers/*.ts apps/desktop/src/lib/trpc/routers/*/*.ts 2>/dev/null | head -3Repository: superset-sh/superset
Length of output: 222
🏁 Script executed:
head -20 apps/desktop/src/lib/trpc/routers/external/index.tsRepository: superset-sh/superset
Length of output: 561
🏁 Script executed:
grep -A 5 "throw new TRPCError" apps/desktop/src/lib/trpc/routers/external/index.ts | head -20Repository: superset-sh/superset
Length of output: 172
🏁 Script executed:
grep -B 5 -A 3 "NOT_FOUND\|NOT_IMPLEMENTED\|FORBIDDEN" apps/desktop/src/lib/trpc/routers/*/index.ts apps/desktop/src/lib/trpc/routers/*/*/index.ts 2>/dev/null | head -40Repository: superset-sh/superset
Length of output: 1508
🏁 Script executed:
wc -l apps/desktop/src/lib/trpc/routers/filesystem/index.tsRepository: superset-sh/superset
Length of output: 121
🏁 Script executed:
grep -B 3 -A 8 "catch" apps/desktop/src/lib/trpc/routers/filesystem/index.tsRepository: superset-sh/superset
Length of output: 2902
🏁 Script executed:
grep -n "\.query\|\.mutation" apps/desktop/src/lib/trpc/routers/filesystem/index.ts | head -20Repository: superset-sh/superset
Length of output: 438
Avoid silently swallowing filesystem errors in exists and stat.
The exists procedure uses a bare catch block that returns false for all errors, including permission denied and IO failures. The stat procedure logs the error but returns null, both of which violate the error handling guidelines.
For exists, differentiate ENOENT (file not found—return false) from other errors (throw TRPCError with INTERNAL_SERVER_ERROR). For stat, throw TRPCError with NOT_FOUND instead of returning null.
🔧 Suggested pattern
+import { TRPCError } from "@trpc/server";
...
} catch (error) {
- return { exists: false, isDirectory: false, isFile: false };
+ if ((error as NodeJS.ErrnoException)?.code === "ENOENT") {
+ return { exists: false, isDirectory: false, isFile: false };
+ }
+ console.error("[filesystem/exists] Failed:", { path: input.path, error });
+ throw new TRPCError({ code: "INTERNAL_SERVER_ERROR" });
}} catch (error) {
console.error("[filesystem/stat] Failed:", {
path: input.path,
error,
});
- return null;
+ throw new TRPCError({
+ code: "NOT_FOUND",
+ message: `Path not found: ${input.path}`,
+ });
}🤖 Prompt for AI Agents
In `@apps/desktop/src/lib/trpc/routers/filesystem/index.ts` around lines 247 -
283, The exists and stat procedures currently swallow filesystem errors; update
exists (the publicProcedure named exists that calls fs.access and fs.stat) to
catch errors, check error.code === 'ENOENT' and return { exists: false, ... }
only for that case, but rethrow other errors as a TRPCError with code
'INTERNAL_SERVER_ERROR' and include the original error message; update stat (the
publicProcedure named stat that calls fs.stat) to stop returning null on failure
and instead throw a TRPCError with code 'NOT_FOUND' (include path and original
error in the message or meta) so callers receive proper error semantics.
| const targetPath = node?.path ?? worktreePath; | ||
| const parentPath = node?.isDirectory ? node.path : worktreePath; | ||
|
|
||
| const { copyPath, copyRelativePath, revealInFinder, openInEditor } = | ||
| usePathActions({ | ||
| absolutePath: targetPath, | ||
| relativePath: node?.relativePath, | ||
| cwd: worktreePath, | ||
| }); | ||
|
|
||
| return ( | ||
| <ContextMenu> | ||
| <ContextMenuTrigger asChild className="flex-1 min-h-0"> | ||
| {children} | ||
| </ContextMenuTrigger> | ||
| <ContextMenuContent className="w-48"> | ||
| <ContextMenuItem onClick={() => onNewFile(parentPath)}> | ||
| <LuFile className="mr-2 size-4" /> | ||
| New File | ||
| </ContextMenuItem> | ||
| <ContextMenuItem onClick={() => onNewFolder(parentPath)}> | ||
| <LuFolder className="mr-2 size-4" /> | ||
| New Folder | ||
| </ContextMenuItem> |
There was a problem hiding this comment.
Use the clicked file’s parent directory for new items.
For file nodes, parentPath falls back to the worktree root, so “New File/Folder” from a file context menu creates in the wrong location. Compute the parent dir from node.path (without Node path imports) before calling onNewFile/onNewFolder.
🐛 Suggested fix
+const getParentPath = (path: string) => path.replace(/[/\\][^/\\]+$/, "");
+
export function FileTreeContextMenu({
children,
node,
worktreePath,
onNewFile,
onNewFolder,
onRename,
onDelete,
}: FileTreeContextMenuProps) {
const targetPath = node?.path ?? worktreePath;
- const parentPath = node?.isDirectory ? node.path : worktreePath;
+ const parentPath = node
+ ? node.isDirectory
+ ? node.path
+ : getParentPath(node.path) || worktreePath
+ : worktreePath;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const targetPath = node?.path ?? worktreePath; | |
| const parentPath = node?.isDirectory ? node.path : worktreePath; | |
| const { copyPath, copyRelativePath, revealInFinder, openInEditor } = | |
| usePathActions({ | |
| absolutePath: targetPath, | |
| relativePath: node?.relativePath, | |
| cwd: worktreePath, | |
| }); | |
| return ( | |
| <ContextMenu> | |
| <ContextMenuTrigger asChild className="flex-1 min-h-0"> | |
| {children} | |
| </ContextMenuTrigger> | |
| <ContextMenuContent className="w-48"> | |
| <ContextMenuItem onClick={() => onNewFile(parentPath)}> | |
| <LuFile className="mr-2 size-4" /> | |
| New File | |
| </ContextMenuItem> | |
| <ContextMenuItem onClick={() => onNewFolder(parentPath)}> | |
| <LuFolder className="mr-2 size-4" /> | |
| New Folder | |
| </ContextMenuItem> | |
| const getParentPath = (path: string) => path.replace(/[/\\][^/\\]+$/, ""); | |
| export function FileTreeContextMenu({ | |
| children, | |
| node, | |
| worktreePath, | |
| onNewFile, | |
| onNewFolder, | |
| onRename, | |
| onDelete, | |
| }: FileTreeContextMenuProps) { | |
| const targetPath = node?.path ?? worktreePath; | |
| const parentPath = node | |
| ? node.isDirectory | |
| ? node.path | |
| : getParentPath(node.path) || worktreePath | |
| : worktreePath; | |
| const { copyPath, copyRelativePath, revealInFinder, openInEditor } = | |
| usePathActions({ | |
| absolutePath: targetPath, | |
| relativePath: node?.relativePath, | |
| cwd: worktreePath, | |
| }); | |
| return ( | |
| <ContextMenu> | |
| <ContextMenuTrigger asChild className="flex-1 min-h-0"> | |
| {children} | |
| </ContextMenuTrigger> | |
| <ContextMenuContent className="w-48"> | |
| <ContextMenuItem onClick={() => onNewFile(parentPath)}> | |
| <LuFile className="mr-2 size-4" /> | |
| New File | |
| </ContextMenuItem> | |
| <ContextMenuItem onClick={() => onNewFolder(parentPath)}> | |
| <LuFolder className="mr-2 size-4" /> | |
| New Folder | |
| </ContextMenuItem> |
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/components/FileTreeContextMenu/FileTreeContextMenu.tsx`
around lines 40 - 63, The context menu uses parentPath which currently falls
back to worktree root for files; compute the parent directory from node.path for
file nodes and only fall back to worktreePath when there is no parent. Update
the logic around parentPath (used by onNewFile/onNewFolder) so: if
node?.isDirectory use node.path, else if node?.path compute its directory by
removing the trailing segment (e.g., slice up to last '/' or path separator)
without importing Node's path module, and otherwise use worktreePath; keep
targetPath and usePathActions unchanged.
| const handleSearchChange = useCallback( | ||
| (e: React.ChangeEvent<HTMLInputElement>) => { | ||
| const value = e.target.value; | ||
| setLocalSearchTerm(value); | ||
|
|
||
| const timeoutId = setTimeout(() => { | ||
| onSearchChange(value); | ||
| }, SEARCH_DEBOUNCE_MS); | ||
|
|
||
| return () => clearTimeout(timeoutId); | ||
| }, | ||
| [onSearchChange], | ||
| ); |
There was a problem hiding this comment.
Broken debounce implementation — cleanup function is never executed.
The returned cleanup function () => clearTimeout(timeoutId) is never called because React event handlers don't use callback return values. Each keystroke creates a new timeout without clearing the previous one, so all callbacks will eventually fire.
🔧 Proposed fix using useRef to track the timeout
+import { useCallback, useState, useRef, useEffect } from "react";
-import { useCallback, useState } from "react";
...
export function FileTreeToolbar({
...
}: FileTreeToolbarProps) {
const [localSearchTerm, setLocalSearchTerm] = useState(searchTerm);
+ const timeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);
+
+ useEffect(() => {
+ return () => {
+ if (timeoutRef.current) {
+ clearTimeout(timeoutRef.current);
+ }
+ };
+ }, []);
const handleSearchChange = useCallback(
(e: React.ChangeEvent<HTMLInputElement>) => {
const value = e.target.value;
setLocalSearchTerm(value);
- const timeoutId = setTimeout(() => {
+ if (timeoutRef.current) {
+ clearTimeout(timeoutRef.current);
+ }
+ timeoutRef.current = setTimeout(() => {
onSearchChange(value);
}, SEARCH_DEBOUNCE_MS);
-
- return () => clearTimeout(timeoutId);
},
[onSearchChange],
);🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/components/FileTreeToolbar/FileTreeToolbar.tsx`
around lines 40 - 52, The debounce in handleSearchChange is broken because the
timeoutId is scoped inside the handler and its cleanup is never invoked; change
to store the timer in a ref (e.g., timeoutRef) and, inside handleSearchChange
(used in FileTreeToolbar), clear timeoutRef.current before assigning a new
setTimeout with SEARCH_DEBOUNCE_MS and call onSearchChange when it fires; also
add a useEffect cleanup that clears timeoutRef.current on unmount to avoid stray
callbacks.
| interface NewItemInputProps { | ||
| mode: NewItemMode; | ||
| parentPath: string; | ||
| onSubmit: (name: string) => void; | ||
| onCancel: () => void; | ||
| level?: number; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "NewItemInput" --type=tsx -A 5 -B 2Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
cat -n apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/components/NewItemInput/NewItemInput.tsxRepository: superset-sh/superset
Length of output: 2200
🏁 Script executed:
rg -n "NewItemInput" -A 5 -B 2Repository: superset-sh/superset
Length of output: 5876
Remove unused parentPath prop from the interface and component signature.
The parentPath prop is declared in NewItemInputProps and passed by the parent component, but it's never used within the NewItemInput component (destructured as _parentPath on line 16). Remove it from both the interface and the function signature to reduce API surface confusion.
♻️ Suggested fix: Remove unused prop
interface NewItemInputProps {
mode: NewItemMode;
- parentPath: string;
onSubmit: (name: string) => void;
onCancel: () => void;
level?: number;
}
export function NewItemInput({
mode,
- parentPath: _parentPath,
onSubmit,
onCancel,
level = 0,
}: NewItemInputProps) {Also remove parentPath={newItemParentPath} from the parent component call in FilesView.tsx (line 317).
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/components/NewItemInput/NewItemInput.tsx`
around lines 6 - 12, Remove the unused parentPath prop: delete parentPath from
the NewItemInputProps interface and remove the parentPath parameter from the
NewItemInput function signature (currently destructured as _parentPath). Update
any internal references (there should be none) and adjust callers by removing
parentPath={newItemParentPath} from the FilesView call site so the prop is no
longer passed. Ensure TypeScript compiles and run linting to catch any leftover
references.
| const handleRename = useCallback( | ||
| ({ id, name }: { id: string; name: string }) => { | ||
| const node = treeData.find((n) => n.id === id); | ||
| if (node) { | ||
| rename(node.path, name); | ||
| } | ||
| }, | ||
| [treeData, rename], | ||
| ); |
There was a problem hiding this comment.
Rename only works for root nodes right now.
treeData.find only checks top-level items, so renaming nested files/folders silently no-ops. Use a recursive lookup (or an index) to resolve nodes anywhere in the tree before calling rename.
🐛 Suggested fix
+const findNodeById = (
+ nodes: FileTreeNodeType[],
+ id: string,
+): FileTreeNodeType | undefined => {
+ for (const node of nodes) {
+ if (node.id === id) return node;
+ if (node.children) {
+ const found = findNodeById(node.children, id);
+ if (found) return found;
+ }
+ }
+ return undefined;
+};
+
export function FilesView() {
@@
const handleRename = useCallback(
({ id, name }: { id: string; name: string }) => {
- const node = treeData.find((n) => n.id === id);
+ const node = findNodeById(treeData, id);
if (node) {
rename(node.path, name);
}
},
[treeData, rename],
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleRename = useCallback( | |
| ({ id, name }: { id: string; name: string }) => { | |
| const node = treeData.find((n) => n.id === id); | |
| if (node) { | |
| rename(node.path, name); | |
| } | |
| }, | |
| [treeData, rename], | |
| ); | |
| const findNodeById = ( | |
| nodes: FileTreeNodeType[], | |
| id: string, | |
| ): FileTreeNodeType | undefined => { | |
| for (const node of nodes) { | |
| if (node.id === id) return node; | |
| if (node.children) { | |
| const found = findNodeById(node.children, id); | |
| if (found) return found; | |
| } | |
| } | |
| return undefined; | |
| }; | |
| export function FilesView() { | |
| const handleRename = useCallback( | |
| ({ id, name }: { id: string; name: string }) => { | |
| const node = findNodeById(treeData, id); | |
| if (node) { | |
| rename(node.path, name); | |
| } | |
| }, | |
| [treeData, rename], | |
| ); |
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/FilesView.tsx`
around lines 187 - 195, handleRename currently uses treeData.find which only
checks top-level nodes so nested items never get found; replace that lookup with
a recursive search (e.g., implement a helper like findNodeById that walks
treeData children) or use an existing index to resolve a node by id anywhere in
the tree, then call rename(node.path, name) as before; update handleRename to
call this recursive helper (referencing handleRename, treeData, findNodeById,
and rename) so nested files/folders are correctly located and renamed.
| const { showHiddenFiles, expandedFolders } = useFileExplorerStore(); | ||
| const currentExpandedFolders = worktreePath | ||
| ? expandedFolders[worktreePath] || [] | ||
| : []; | ||
|
|
||
| const trpcUtils = electronTrpc.useUtils(); | ||
|
|
||
| const { | ||
| data: rootEntries, | ||
| isLoading, | ||
| error, | ||
| refetch, | ||
| } = electronTrpc.filesystem.readDirectory.useQuery( | ||
| { | ||
| dirPath: worktreePath || "", | ||
| rootPath: worktreePath || "", | ||
| includeHidden: showHiddenFiles, | ||
| }, | ||
| { | ||
| enabled: !!worktreePath, | ||
| staleTime: 5000, | ||
| }, | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
wc -l apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/hooks/useFileTree.tsRepository: superset-sh/superset
Length of output: 175
🏁 Script executed:
cat -n apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/hooks/useFileTree.tsRepository: superset-sh/superset
Length of output: 4199
Clear childrenCache when worktreePath or showHiddenFiles changes; use params object for loadChildren; extract magic number.
The cache at lines 22-24 is only cleared in the explicit refetch() call but not when showHiddenFiles toggles or worktreePath changes. This causes stale expanded-folder data to persist even after the root query re-runs with new parameters—users may see hidden files in cached children after disabling them.
Additionally, loadChildren at line 91 violates the params-object guideline (should accept { nodeId, nodePath } instead of positional args), and the hardcoded 5000 staleTime at line 46 should be extracted to a named constant.
Suggested fix
+const STALE_TIME_MS = 5000;
const trpcUtils = electronTrpc.useUtils();
const {
@@ -42,7 +44,7 @@
includeHidden: showHiddenFiles,
},
{
enabled: !!worktreePath,
- staleTime: 5000,
+ staleTime: STALE_TIME_MS,
},
);And add cache invalidation before the trpcUtils assignment:
+useEffect(() => {
+ setChildrenCache({});
+}, [worktreePath, showHiddenFiles]);
+
const trpcUtils = electronTrpc.useUtils();For loadChildren, change signature to accept a params object:
-const loadChildren = useCallback(
- async (nodeId: string, nodePath: string): Promise<FileTreeNode[]> => {
+const loadChildren = useCallback(
+ async ({ nodeId, nodePath }: { nodeId: string; nodePath: string }): Promise<FileTreeNode[]> => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { showHiddenFiles, expandedFolders } = useFileExplorerStore(); | |
| const currentExpandedFolders = worktreePath | |
| ? expandedFolders[worktreePath] || [] | |
| : []; | |
| const trpcUtils = electronTrpc.useUtils(); | |
| const { | |
| data: rootEntries, | |
| isLoading, | |
| error, | |
| refetch, | |
| } = electronTrpc.filesystem.readDirectory.useQuery( | |
| { | |
| dirPath: worktreePath || "", | |
| rootPath: worktreePath || "", | |
| includeHidden: showHiddenFiles, | |
| }, | |
| { | |
| enabled: !!worktreePath, | |
| staleTime: 5000, | |
| }, | |
| ); | |
| const STALE_TIME_MS = 5000; | |
| const { showHiddenFiles, expandedFolders } = useFileExplorerStore(); | |
| const currentExpandedFolders = worktreePath | |
| ? expandedFolders[worktreePath] || [] | |
| : []; | |
| useEffect(() => { | |
| setChildrenCache({}); | |
| }, [worktreePath, showHiddenFiles]); | |
| const trpcUtils = electronTrpc.useUtils(); | |
| const { | |
| data: rootEntries, | |
| isLoading, | |
| error, | |
| refetch, | |
| } = electronTrpc.filesystem.readDirectory.useQuery( | |
| { | |
| dirPath: worktreePath || "", | |
| rootPath: worktreePath || "", | |
| includeHidden: showHiddenFiles, | |
| }, | |
| { | |
| enabled: !!worktreePath, | |
| staleTime: STALE_TIME_MS, | |
| }, | |
| ); |
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/hooks/useFileTree.ts`
around lines 26 - 48, The childrenCache needs to be invalidated when
worktreePath or showHiddenFiles change: add a useEffect that clears
childrenCache (the in-module cache used for expanded folder children) whenever
worktreePath or showHiddenFiles updates, and perform that invalidation before
you create trpcUtils to ensure subsequent queries use fresh state; change
loadChildren to accept a params object ({ nodeId, nodePath }) instead of
positional args and update any internal callers to pass that object; and extract
the hardcoded staleTime value (5000) from the
electronTrpc.filesystem.readDirectory.useQuery options into a named constant
(e.g., DIRECTORY_STALE_TIME) and use that constant in the query options.
1 participant
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.