feat(desktop): show existing disk worktrees in Open Workspace modal#1154
Conversation
Add support for discovering and opening git worktrees that exist on disk but aren't tracked in the app's database. This allows users to open worktrees created via git CLI directly from the "Existing" tab.
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds discovery and opening of external Git worktrees: backend TRPC procedures to list and open disk-based worktrees, a git utility to parse worktree state, and frontend hooks/components to display, search, and open external worktrees with setup-command handling and workspace creation/attachment. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant UI as NewWorkspaceModal UI
participant TRPC as TRPC Router
participant Git as Git Utility
participant DB as Workspace DB
participant Tab as Tab Manager
participant Terminal as Terminal
User->>UI: open modal / request worktrees
UI->>TRPC: getExternalWorktrees(projectId)
TRPC->>Git: listExternalWorktrees(mainRepoPath)
Git-->>TRPC: ExternalWorktree[]
TRPC->>DB: Query tracked worktrees
TRPC-->>UI: Filtered external worktrees
UI->>User: display searchable list
User->>UI: select external worktree to open
UI->>TRPC: openExternalWorktree({projectId, worktreePath, branch})
TRPC->>Git: verify worktree exists on disk
TRPC->>DB: check/create worktree entry and workspace
TRPC->>DB: load setup commands / copy config
TRPC-->>UI: workspace + initialCommands + wasExisting
UI->>Tab: open/create workspace tab
UI->>Terminal: attach terminal + run initialCommands
UI->>User: navigate to workspace
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/components/DiskWorktreesSection.tsx`:
- Line 29: The folderName computation uses wt.path.split("/") which fails on
Windows backslashes; normalize backslashes to forward slashes before splitting
(e.g., replace backslashes with "/") so folderName =
(normalizedPath.split("/").pop() ?? wt.branch); update the logic in
DiskWorktreesSection (where folderName is derived from wt.path) to mirror the
defensive normalization used in FileSearchResultItem.tsx and InitGitDialog.tsx.
🧹 Nitpick comments (8)
apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts (1)
770-772: Use prefixed logging pattern for consistency.The error log should follow the
[domain/operation] messagepattern as specified in coding guidelines.Suggested fix
} catch (error) { - console.error(`Failed to list disk worktrees: ${error}`); + console.error(`[git/listDiskWorktrees] Failed to list disk worktrees: ${error}`); throw error; }apps/desktop/src/lib/trpc/routers/workspaces/procedures/git-status.ts (1)
174-177: Consider throwingTRPCErrorfor missing project.Returning an empty array when the project doesn't exist silently masks the error. Other procedures in this file throw errors when the project is not found (e.g.,
refreshGitStatus). Consider throwing aTRPCErrorwith codeNOT_FOUNDfor consistency.Alternative approach
const project = getProject(input.projectId); if (!project) { - return []; + throw new TRPCError({ + code: "NOT_FOUND", + message: `Project ${input.projectId} not found`, + }); }Note: This would require importing
TRPCErrorfrom@trpc/server.As per coding guidelines: Use
TRPCErrorwith codeNOT_FOUNDwhen a resource doesn't exist.apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts (2)
627-638: UseTRPCErrorfor consistent error handling.The procedure throws plain
Errorfor "not found" cases, but tRPC procedures should useTRPCErrorwith appropriate codes for consistent API behavior.Suggested fix
Add import at top of file:
import { TRPCError } from "@trpc/server";Then update the error handling:
const project = getProject(input.projectId); if (!project) { - throw new Error(`Project ${input.projectId} not found`); + throw new TRPCError({ + code: "NOT_FOUND", + message: `Project ${input.projectId} not found`, + }); } const exists = await worktreeExists( project.mainRepoPath, input.worktreePath, ); if (!exists) { - throw new Error("Worktree no longer exists on disk"); + throw new TRPCError({ + code: "NOT_FOUND", + message: "Worktree no longer exists on disk", + }); }As per coding guidelines: Use
TRPCErrorwith codeNOT_FOUNDwhen a resource doesn't exist.
670-702: Consider extracting shared workspace creation logic.The workspace creation logic (get max tab order, insert workspace, set last active, activate project, load setup config, track event) is duplicated between the "existing worktree" branch (lines 670-702) and the "new worktree" branch (lines 718-750). This is also similar to
openWorktreeprocedure (lines 584-615).A helper function could reduce duplication:
function createWorkspaceForWorktree({ projectId, worktreeId, branch, project, source, }: { projectId: string; worktreeId: string; branch: string; project: typeof projects.$inferSelect; source: string; }) { const maxTabOrder = getMaxWorkspaceTabOrder(projectId); const workspace = localDb .insert(workspaces) .values({ projectId, worktreeId, type: "worktree", branch, name: branch, tabOrder: maxTabOrder + 1, }) .returning() .get(); setLastActiveWorkspace(workspace.id); activateProject(project); const setupConfig = loadSetupConfig(project.mainRepoPath); return { workspace, setupConfig }; }Also applies to: 705-750
apps/desktop/src/renderer/react-query/workspaces/useOpenDiskWorktree.ts (1)
37-42: Consider handling potential mutation errors.The
createOrAttach.mutatecall is fire-and-forget. If terminal creation fails, the user won't be notified. Consider usingmutateAsyncwith error handling or adding anonErrorcallback to the mutation.This appears consistent with the pattern in
useOpenWorktree, so it may be intentional. However, for better UX, you could handle errors:createOrAttach.mutate( { paneId, tabId, workspaceId: data.workspace.id, initialCommands, }, { onError: (error) => { toast.error("Failed to initialize terminal", { description: error.message, }); }, } );apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/components/DiskWorktreesSection.tsx (3)
3-6: Consider exportingDiskWorktreeinterface if used elsewhere.If this type is consumed by parent components or the hook layer (e.g.,
useOpenDiskWorktree), export it to avoid type duplication.♻️ Suggested change
-interface DiskWorktree { +export interface DiskWorktree { path: string; branch: string; }
10-10: Consider using a params object for the callback.Per coding guidelines, functions with 2+ parameters should accept a single params object. This improves readability and extensibility.
♻️ Suggested change
interface DiskWorktreesSectionProps { diskWorktrees: DiskWorktree[]; - onOpenWorktree: (path: string, branch: string) => void; + onOpenWorktree: (params: { path: string; branch: string }) => void; disabled: boolean; }Then update the call site at line 34:
-onClick={() => onOpenWorktree(wt.path, wt.branch)} +onClick={() => onOpenWorktree({ path: wt.path, branch: wt.branch })}
19-26: Consider handling the empty state.When
diskWorktreesis empty, the component still renders the "Disk Worktrees" header with the border, which may look odd. If the parent component doesn't filter this case, consider returningnullearly.♻️ Suggested change
}: DiskWorktreesSectionProps) { + if (diskWorktrees.length === 0) { + return null; + } + return ( <div className="space-y-1">
| </div> | ||
|
|
||
| {diskWorktrees.map((wt) => { | ||
| const folderName = wt.path.split("/").pop() ?? wt.branch; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, read the component file to see the full context
cat -n apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/components/DiskWorktreesSection.tsxRepository: superset-sh/superset
Length of output: 1834
🏁 Script executed:
# Search for where wt (worktree) is defined and what type it is
rg -n "diskWorktrees|DiskWorktree" apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/components/DiskWorktreesSection.tsx -B 5 -A 5Repository: superset-sh/superset
Length of output: 962
🏁 Script executed:
# Find git utility functions that provide worktree paths
fd "git" apps/desktop/src -type f -name "*.ts" | head -20Repository: superset-sh/superset
Length of output: 234
🏁 Script executed:
# Search for listDiskWorktrees or similar function
rg -n "listDiskWorktrees|getDiskWorktrees" --type tsRepository: superset-sh/superset
Length of output: 406
🏁 Script executed:
# Read the git.ts file around listDiskWorktrees function
sed -n '730,800p' apps/desktop/src/lib/trpc/routers/workspaces/utils/git.tsRepository: superset-sh/superset
Length of output: 2114
🏁 Script executed:
# Search for git worktree list command usage
rg -n "git.*worktree.*list" apps/desktop/src -i -A 5 -B 2Repository: superset-sh/superset
Length of output: 3043
🏁 Script executed:
# Search for any path normalization or platform-specific handling in git.ts
rg -n "normalize|windows|platform|backslash" apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts -iRepository: superset-sh/superset
Length of output: 362
🏁 Script executed:
# Check if there are any other instances of path splitting in the codebase and how they handle it
rg -n 'split\s*\(' apps/desktop/src/renderer/components/NewWorkspaceModal -B 2 -A 2Repository: superset-sh/superset
Length of output: 819
🏁 Script executed:
# Check the DiskWorktree interface definition more broadly
rg -n "interface DiskWorktree" apps/desktop/src -B 2 -A 5Repository: superset-sh/superset
Length of output: 2577
🏁 Script executed:
# Search for any comments about git path normalization
rg -n "git.*path|normalize.*path" apps/desktop/src -i -B 2 -A 2Repository: superset-sh/superset
Length of output: 50376
Use defensive path splitting to normalize Windows backslashes.
While git CLI normalizes paths to forward slashes across platforms, the codebase applies explicit normalization in similar scenarios (see FileSearchResultItem.tsx line 38 and InitGitDialog.tsx line 8). For consistency and robustness, normalize the path before splitting.
🛠️ Suggested fix
-const folderName = wt.path.split("/").pop() ?? wt.branch;
+const folderName = wt.path.replace(/\\/g, "/").split("/").pop() ?? wt.branch;🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/components/DiskWorktreesSection.tsx`
at line 29, The folderName computation uses wt.path.split("/") which fails on
Windows backslashes; normalize backslashes to forward slashes before splitting
(e.g., replace backslashes with "/") so folderName =
(normalizedPath.split("/").pop() ?? wt.branch); update the logic in
DiskWorktreesSection (where folderName is derived from wt.path) to mirror the
defensive normalization used in FileSearchResultItem.tsx and InitGitDialog.tsx.
Use smaller text size and show the full path on a separate line below the branch name for better readability and proper truncation.
Update WorktreesSection and DiskWorktreesSection to use popover/command pattern like BranchesSection for consistent UI. Shows branch name and path on separate lines with proper truncation and search functionality.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/components/DiskWorktreesSection.tsx`:
- Around line 13-41: The DiskWorktree type and DiskWorktreesSection currently
assume branch is always a string which causes toLowerCase() and rendering to
crash for detached worktrees; update the DiskWorktree interface to allow branch:
string | null, change the onOpenWorktree callback signature to accept branch:
string | null, and in DiskWorktreesSection (particularly where filteredWorktrees
is computed and where branch is rendered/used) guard against null by using a
safe fallback (e.g., '' for filtering and a label like 'detached' for display)
so null branches won’t call toLowerCase() and callers receive the nullable
branch value to handle.
🧹 Nitpick comments (3)
apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/components/WorktreesSection.tsx (1)
44-49: Includepathin worktree search matching.At Lines 45–48 the filter only matches
branch, but the UI now displayspath, so users can’t find by location. Consider filtering onpathtoo (consistent with Disk Worktrees).♻️ Proposed tweak
- const filteredWorktrees = searchValue - ? allWorktrees.filter((wt) => - wt.branch.toLowerCase().includes(searchValue.toLowerCase()), - ) - : allWorktrees; + const searchLower = searchValue.toLowerCase(); + const filteredWorktrees = searchValue + ? allWorktrees.filter( + (wt) => + wt.branch.toLowerCase().includes(searchLower) || + wt.path.toLowerCase().includes(searchLower), + ) + : allWorktrees;apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/components/DiskWorktreesSection.tsx (1)
1-12: Align component file layout with the components convention.This component sits directly under
components/…/components. The guideline calls for one component per folder (ComponentName/ComponentName.tsx) with a barrel export inindex.ts. Consider moving this file toDiskWorktreesSection/DiskWorktreesSection.tsxand updating the barrel export.As per coding guidelines, Use folder structure with one component per file:
ComponentName/ComponentName.tsxwith barrel export inindex.ts.apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/ExistingWorktreesList.tsx (1)
68-70: Use params objects for open handlers to avoid positional args.Lines 68 and 135 define handlers with two positional arguments. This is easy to mix up and conflicts with the params-object guideline. Consider switching to a single params object and update the
WorktreesSection/DiskWorktreesSectioncallbacks accordingly.As per coding guidelines, Functions with 2+ parameters should accept a single params object with named properties instead of positional arguments.♻️ Proposed refactor
- const handleOpenWorktree = async (worktreeId: string, branch: string) => { + const handleOpenWorktree = async ({ + worktreeId, + branch, + }: { + worktreeId: string; + branch: string; + }) => { setWorktreeOpen(false); setWorktreeSearch(""); toast.promise(openWorktree.mutateAsync({ worktreeId }), { loading: "Opening workspace...", success: () => { onOpenSuccess(); return `Opened ${branch}`; }, error: (err) => err instanceof Error ? err.message : "Failed to open workspace", }); }; - const handleOpenDiskWorktree = async (path: string, branch: string) => { + const handleOpenDiskWorktree = async ({ + worktreePath, + branch, + }: { + worktreePath: string; + branch: string | null; + }) => { setDiskWorktreeOpen(false); setDiskWorktreeSearch(""); toast.promise( - openDiskWorktree.mutateAsync({ projectId, worktreePath: path, branch }), + openDiskWorktree.mutateAsync({ + projectId, + worktreePath, + branch, + }), { loading: "Opening workspace...", success: () => { onOpenSuccess(); return `Opened ${branch}`; }, error: (err) => err instanceof Error ? err.message : "Failed to open workspace", }, ); };Also applies to: 135-150
| interface DiskWorktree { | ||
| path: string; | ||
| branch: string; | ||
| } | ||
|
|
||
| interface DiskWorktreesSectionProps { | ||
| diskWorktrees: DiskWorktree[]; | ||
| searchValue: string; | ||
| onSearchChange: (value: string) => void; | ||
| isOpen: boolean; | ||
| onOpenChange: (open: boolean) => void; | ||
| onOpenWorktree: (path: string, branch: string) => void; | ||
| disabled: boolean; | ||
| } | ||
|
|
||
| export function DiskWorktreesSection({ | ||
| diskWorktrees, | ||
| searchValue, | ||
| onSearchChange, | ||
| isOpen, | ||
| onOpenChange, | ||
| onOpenWorktree, | ||
| disabled, | ||
| }: DiskWorktreesSectionProps) { | ||
| const filteredWorktrees = searchValue | ||
| ? diskWorktrees.filter( | ||
| (wt) => | ||
| wt.branch.toLowerCase().includes(searchValue.toLowerCase()) || | ||
| wt.path.toLowerCase().includes(searchValue.toLowerCase()), |
There was a problem hiding this comment.
Guard against detached worktrees where branch is null.
The backend DiskWorktree type allows branch: string | null (detached worktrees). At Lines 40–41 and 91–92, toLowerCase() and rendering will throw if branch is null. Please accept nullable branches and provide a fallback label, plus propagate nullability to the callback so callers can handle it.
🛠️ Suggested hardening
interface DiskWorktree {
path: string;
- branch: string;
+ branch: string | null;
}
interface DiskWorktreesSectionProps {
diskWorktrees: DiskWorktree[];
searchValue: string;
onSearchChange: (value: string) => void;
isOpen: boolean;
onOpenChange: (open: boolean) => void;
- onOpenWorktree: (path: string, branch: string) => void;
+ onOpenWorktree: (path: string, branch: string | null) => void;
disabled: boolean;
}
- const filteredWorktrees = searchValue
- ? diskWorktrees.filter(
- (wt) =>
- wt.branch.toLowerCase().includes(searchValue.toLowerCase()) ||
- wt.path.toLowerCase().includes(searchValue.toLowerCase()),
- )
- : diskWorktrees;
+ const searchLower = searchValue.toLowerCase();
+ const filteredWorktrees = searchValue
+ ? diskWorktrees.filter((wt) => {
+ const branch = wt.branch ?? "";
+ return (
+ branch.toLowerCase().includes(searchLower) ||
+ wt.path.toLowerCase().includes(searchLower)
+ );
+ })
+ : diskWorktrees;
...
{filteredWorktrees.map((wt) => (
+ const branchLabel = wt.branch ?? "(detached)";
<CommandItem
key={wt.path}
value={wt.path}
- onSelect={() => onOpenWorktree(wt.path, wt.branch)}
+ onSelect={() => onOpenWorktree(wt.path, wt.branch)}
className="flex flex-col items-start gap-0.5"
>
<span className="flex items-center gap-2 w-full">
<LuGitBranch className="size-3.5 shrink-0 text-muted-foreground" />
<span className="flex-1 truncate text-xs font-mono">
- {wt.branch}
+ {branchLabel}
</span>
</span>🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/components/DiskWorktreesSection.tsx`
around lines 13 - 41, The DiskWorktree type and DiskWorktreesSection currently
assume branch is always a string which causes toLowerCase() and rendering to
crash for detached worktrees; update the DiskWorktree interface to allow branch:
string | null, change the onOpenWorktree callback signature to accept branch:
string | null, and in DiskWorktreesSection (particularly where filteredWorktrees
is computed and where branch is rendered/used) guard against null by using a
safe fallback (e.g., '' for filtering and a label like 'detached' for display)
so null branches won’t call toLowerCase() and callers receive the nullable
branch value to handle.
- Worktrees → Superset Worktrees - Disk Worktrees → External Worktrees
Merge Superset Worktrees and External Worktrees into a unified "Worktrees" dropdown with grouped items inside. This reduces visual clutter from 4 sections to 3 in the Existing tab (PR, Branches, Worktrees).
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/components/WorktreesSection.tsx`:
- Around line 154-175: WorktreesSection renders wt.branch and passes it into
onOpenDiskWorktree without null checks; update the rendering and callback to
handle nullable branches by using a safe fallback (e.g. show "(detached)" or
"(no branch)" for display) and pass a stable value (empty string or undefined)
to onOpenDiskWorktree when wt.branch is null, and ensure onOpenDiskWorktree's
signature accepts a nullable/optional branch; change occurrences of wt.branch in
the CommandItem label and the onSelect call accordingly so null won't cause
runtime errors.
- Around line 71-77: The filter for filteredDisk calls wt.branch.toLowerCase()
which will throw for detached HEAD worktrees where branch can be null; update
the predicate in the diskWorktrees filter used to compute filteredDisk to guard
against null branch (e.g., use optional chaining or coerce branch to an empty
string before toLowerCase()) and ensure the comparison uses the same searchLower
variable for both branch and path checks; reference wt.branch, diskWorktrees,
filteredDisk and searchLower in your change so the null case is safely handled.
| const filteredDisk = searchValue | ||
| ? diskWorktrees.filter( | ||
| (wt) => | ||
| wt.branch.toLowerCase().includes(searchLower) || | ||
| wt.path.toLowerCase().includes(searchLower), | ||
| ) | ||
| : diskWorktrees; |
There was a problem hiding this comment.
Null-unsafe filter will crash on detached HEAD worktrees.
wt.branch.toLowerCase() will throw if branch is null. This is directly tied to the type mismatch noted above.
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/components/WorktreesSection.tsx`
around lines 71 - 77, The filter for filteredDisk calls wt.branch.toLowerCase()
which will throw for detached HEAD worktrees where branch can be null; update
the predicate in the diskWorktrees filter used to compute filteredDisk to guard
against null branch (e.g., use optional chaining or coerce branch to an empty
string before toLowerCase()) and ensure the comparison uses the same searchLower
variable for both branch and path checks; reference wt.branch, diskWorktrees,
filteredDisk and searchLower in your change so the null case is safely handled.
| {filteredDisk.length > 0 && ( | ||
| <CommandGroup heading="External"> | ||
| {filteredDisk.map((wt) => ( | ||
| <CommandItem | ||
| key={wt.path} | ||
| value={wt.path} | ||
| onSelect={() => onOpenDiskWorktree(wt.path, wt.branch)} | ||
| className="flex flex-col items-start gap-0.5" | ||
| > | ||
| <span className="flex items-center gap-2 w-full"> | ||
| <LuGitBranch className="size-3.5 shrink-0 text-muted-foreground" /> | ||
| <span className="flex-1 truncate text-xs font-mono"> | ||
| {wt.branch} | ||
| </span> | ||
| </span> | ||
| <span className="text-[10px] text-muted-foreground/60 truncate w-full pl-5"> | ||
| {wt.path} | ||
| </span> | ||
| </CommandItem> | ||
| ))} | ||
| </CommandGroup> | ||
| )} |
There was a problem hiding this comment.
Null-unsafe: wt.branch display and callback will fail if branch is null.
Lines 160 and 166 use wt.branch without null checks. If the type mismatch is resolved by accepting null, these need corresponding updates.
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/components/WorktreesSection.tsx`
around lines 154 - 175, WorktreesSection renders wt.branch and passes it into
onOpenDiskWorktree without null checks; update the rendering and callback to
handle nullable branches by using a safe fallback (e.g. show "(detached)" or
"(no branch)" for display) and pass a stable value (empty string or undefined)
to onOpenDiskWorktree when wt.branch is null, and ensure onOpenDiskWorktree's
signature accepts a nullable/optional branch; change occurrences of wt.branch in
the CommandItem label and the onSelect call accordingly so null won't cause
runtime errors.
Branches with external disk worktrees were appearing in both the Branches dropdown and the External worktrees group. Clicking on the branch would try to create a new worktree, causing "Setup incomplete" errors. Now branches are filtered out if they have either a tracked or external worktree.
Imported worktrees were showing "Setup incomplete" because gitStatus was set to null. The workspace page checks for null gitStatus to detect interrupted initialization. For imported worktrees, we now set a valid gitStatus object so they're recognized as ready immediately.
Kitenite
force-pushed
the
kitenite/open-workspace-existing-worktrees
branch
from
f2b4e16 to
613fcc2
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts`:
- Around line 640-644: The worktree lookup (existingWorktree created via
localDb.select().from(worktrees).where(...).get()) only filters by
worktrees.path and must also filter by the project id to avoid cross-project
collisions; update the where clause to include a second predicate comparing
worktrees.projectId (or the correct column name on the worktrees table) to
input.projectId (e.g., combine eq(worktrees.path, input.worktreePath) AND
eq(worktrees.projectId, input.projectId)) so the query only returns a worktree
belonging to the same project before creating the workspace.
🧹 Nitpick comments (1)
apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts (1)
627-638: Consider usingTRPCErrorfor consistent error handling.The guidelines recommend using
TRPCErrorwith appropriate codes (e.g.,NOT_FOUNDfor missing resources). While the rest of this file uses plainError, adoptingTRPCErrorhere would provide better client-side error handling semantics.♻️ Suggested improvement
+import { TRPCError } from "@trpc/server"; const project = getProject(input.projectId); if (!project) { - throw new Error(`Project ${input.projectId} not found`); + throw new TRPCError({ + code: "NOT_FOUND", + message: `Project ${input.projectId} not found`, + }); } const exists = await worktreeExists( project.mainRepoPath, input.worktreePath, ); if (!exists) { - throw new Error("Worktree no longer exists on disk"); + throw new TRPCError({ + code: "NOT_FOUND", + message: "Worktree no longer exists on disk", + }); }As per coding guidelines:
apps/*/src/**/trpc/**/*.{ts,tsx}: UseTRPCErrorwith codeNOT_FOUNDwhen a resource doesn't exist.
Rename all disk worktree references to external worktree for consistency with the UI labeling. Also fix worktree lookup to filter by projectId to prevent cross-project data inconsistencies. Changes: - DiskWorktree -> ExternalWorktree - listDiskWorktrees -> listExternalWorktrees - getUntrackedDiskWorktrees -> getExternalWorktrees - openDiskWorktree -> openExternalWorktree - useOpenDiskWorktree -> useOpenExternalWorktree - disk_import -> external_import (analytics) - Add projectId filter to worktree lookup query
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/desktop/src/lib/trpc/routers/workspaces/procedures/git-status.ts`:
- Around line 171-176: The getExternalWorktrees procedure currently returns an
empty array when getProject(input.projectId) yields no project; instead throw a
TRPCError with code NOT_FOUND so callers can handle the missing resource. In the
getExternalWorktrees publicProcedure, after calling getProject(input.projectId)
(and before returning), replace the empty-array return with throwing new
TRPCError({ code: 'NOT_FOUND', message: `Project ${input.projectId} not found`
}) (import TRPCError if needed) so the procedure surfaces the missing project
correctly.
In `@apps/desktop/src/renderer/react-query/workspaces/useOpenExternalWorktree.ts`:
- Around line 37-53: The createOrAttach.mutate(...) and
dismissConfigToast.mutate(...) calls lack error handling and may fail silently;
update both calls (createOrAttach.mutate and dismissConfigToast.mutate) to pass
an options object with an onError handler that logs the error with a clear
prefix (e.g., "createOrAttach failed:" and "dismissConfigToast failed:"), and
optionally surface a user-facing toast/error notification; ensure the onError
handlers reference the mutation result so failures are visible and diagnosable.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/components/WorktreesSection.tsx (1)
28-38: Prefer a params object for worktree open callbacks.These callbacks use positional args; switching to a params object improves clarity and aligns with the guideline. Update call sites (e.g., ExistingWorktreesList handlers) accordingly.
♻️ Suggested refactor
- onOpenWorktree: (worktreeId: string, branch: string) => void; - onOpenExternalWorktree: (path: string, branch: string) => void; + onOpenWorktree: (params: { worktreeId: string; branch: string }) => void; + onOpenExternalWorktree: (params: { path: string; branch: string }) => void;- onSelect={() => onOpenWorktree(wt.id, wt.branch)} + onSelect={() => + onOpenWorktree({ worktreeId: wt.id, branch: wt.branch }) + }- onSelect={() => onOpenExternalWorktree(wt.path, wt.branch)} + onSelect={() => + onOpenExternalWorktree({ + path: wt.path, + branch: wt.branch, + }) + }As per coding guidelines, Functions with 2+ parameters should accept a single params object with named properties instead of positional arguments.
Also applies to: 130-161
| getExternalWorktrees: publicProcedure | ||
| .input(z.object({ projectId: z.string() })) | ||
| .query(async ({ input }) => { | ||
| const project = getProject(input.projectId); | ||
| if (!project) { | ||
| return []; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /tmp/repo && find . -name "git-status.ts" -path "*/trpc/routers/workspaces/procedures/*" 2>/dev/nullRepository: superset-sh/superset
Length of output: 123
🏁 Script executed:
cd /tmp/repo && git ls-files | grep -E "git-status\.ts$"Repository: superset-sh/superset
Length of output: 123
🏁 Script executed:
cd /tmp/repo && find . -path "*/apps/desktop/src/lib/trpc/routers/workspaces/procedures/git-status.ts" 2>/dev/nullRepository: superset-sh/superset
Length of output: 123
🏁 Script executed:
git ls-files | grep "git-status.ts"Repository: superset-sh/superset
Length of output: 133
🏁 Script executed:
find . -type f -name "git-status.ts" 2>/dev/null | head -20Repository: superset-sh/superset
Length of output: 135
🏁 Script executed:
fd "git-status.ts" 2>/dev/nullRepository: superset-sh/superset
Length of output: 133
🏁 Script executed:
wc -l apps/desktop/src/lib/trpc/routers/workspaces/procedures/git-status.tsRepository: superset-sh/superset
Length of output: 137
🏁 Script executed:
sed -n '1,30p' apps/desktop/src/lib/trpc/routers/workspaces/procedures/git-status.tsRepository: superset-sh/superset
Length of output: 933
🏁 Script executed:
sed -n '165,185p' apps/desktop/src/lib/trpc/routers/workspaces/procedures/git-status.tsRepository: superset-sh/superset
Length of output: 628
🏁 Script executed:
sed -n '1,50p' apps/desktop/src/lib/trpc/routers/workspaces/procedures/git-status.tsRepository: superset-sh/superset
Length of output: 1519
🏁 Script executed:
rg "throw new" apps/desktop/src/lib/trpc/routers/workspaces/procedures/git-status.ts -A 2Repository: superset-sh/superset
Length of output: 308
🏁 Script executed:
rg "TRPCError" apps/desktop/src/lib/trpc/routers/workspaces/procedures/git-status.tsRepository: superset-sh/superset
Length of output: 46
🏁 Script executed:
rg "TRPCError" apps/desktop/src/lib/trpc/ --type ts --type tsx -lRepository: superset-sh/superset
Length of output: 91
🏁 Script executed:
head -5 apps/desktop/src/lib/trpc/routers/workspaces/procedures/git-status.ts && echo "---" && sed -n '20,50p' apps/desktop/src/lib/trpc/routers/workspaces/procedures/git-status.tsRepository: superset-sh/superset
Length of output: 1220
🏁 Script executed:
find apps/desktop/src/lib/trpc/routers -name "*.ts" -o -name "*.tsx" | head -5 | xargs grep -l "TRPCError"Repository: superset-sh/superset
Length of output: 48
🏁 Script executed:
grep -n "import.*TRPCError" apps/desktop/src/lib/trpc/**/*.ts 2>/dev/null | head -10Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
find apps/desktop/src/lib/trpc -name "*.ts" | head -3 | xargs catRepository: superset-sh/superset
Length of output: 4707
🏁 Script executed:
sed -n '1,10p' apps/desktop/src/lib/trpc/index.ts 2>/dev/null || sed -n '1,10p' apps/desktop/src/lib/trpc.ts 2>/dev/null || echo "Checking directory structure..."Repository: superset-sh/superset
Length of output: 377
🏁 Script executed:
ls -la apps/desktop/src/lib/trpc/Repository: superset-sh/superset
Length of output: 293
🏁 Script executed:
cat apps/desktop/src/lib/trpc/routers/workspaces/procedures/git-status.tsRepository: superset-sh/superset
Length of output: 5431
Use TRPCError.NOT_FOUND for missing projects.
Returning an empty list for an invalid projectId hides an error path and bypasses standard tRPC handling. Throw NOT_FOUND so callers can surface the issue properly.
🐛 Suggested fix
+import { TRPCError } from "@trpc/server";
import { z } from "zod";- if (!project) {
- return [];
- }
+ if (!project) {
+ throw new TRPCError({
+ code: "NOT_FOUND",
+ message: `Project ${input.projectId} not found`,
+ });
+ }Per coding guidelines for apps/*/src/**/trpc/**/*.{ts,tsx}, use TRPCError with code NOT_FOUND when a resource doesn't exist.
📝 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.
| getExternalWorktrees: publicProcedure | |
| .input(z.object({ projectId: z.string() })) | |
| .query(async ({ input }) => { | |
| const project = getProject(input.projectId); | |
| if (!project) { | |
| return []; | |
| getExternalWorktrees: publicProcedure | |
| .input(z.object({ projectId: z.string() })) | |
| .query(async ({ input }) => { | |
| const project = getProject(input.projectId); | |
| if (!project) { | |
| throw new TRPCError({ | |
| code: "NOT_FOUND", | |
| message: `Project ${input.projectId} not found`, | |
| }); | |
| } |
🤖 Prompt for AI Agents
In `@apps/desktop/src/lib/trpc/routers/workspaces/procedures/git-status.ts` around
lines 171 - 176, The getExternalWorktrees procedure currently returns an empty
array when getProject(input.projectId) yields no project; instead throw a
TRPCError with code NOT_FOUND so callers can handle the missing resource. In the
getExternalWorktrees publicProcedure, after calling getProject(input.projectId)
(and before returning), replace the empty-array return with throwing new
TRPCError({ code: 'NOT_FOUND', message: `Project ${input.projectId} not found`
}) (import TRPCError if needed) so the procedure surfaces the missing project
correctly.
| createOrAttach.mutate({ | ||
| paneId, | ||
| tabId, | ||
| workspaceId: data.workspace.id, | ||
| initialCommands, | ||
| }); | ||
|
|
||
| if (!initialCommands) { | ||
| toast.info("No setup script configured", { | ||
| description: "Automate workspace setup with a config.json file", | ||
| action: { | ||
| label: "Configure", | ||
| onClick: () => openConfigModal(data.projectId), | ||
| }, | ||
| onDismiss: () => { | ||
| dismissConfigToast.mutate({ projectId: data.projectId }); | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
TanStack Query useMutation mutate onError callback second parameter
💡 Result:
In TanStack Query, the onError callback’s second parameter is the same variables you passed to mutate(variables, …).
- TanStack Query (latest docs):
onError: (err, variables, onMutateResult, context) => void→ 2nd arg =variables. [1] - TanStack Query v4 docs:
onError: (err, variables, context) => void→ 2nd arg =variables. [2]
Example:
mutation.mutate({ id: 123 }, {
onError: (err, variables) => {
// variables === { id: 123 }
},
})Sources: [1] [2]
🏁 Script executed:
cat -n apps/desktop/src/renderer/react-query/workspaces/useOpenExternalWorktree.ts | head -70Repository: superset-sh/superset
Length of output: 2518
Add error handling to mutation calls to prevent silent failures.
The createOrAttach.mutate() (line 37) and dismissConfigToast.mutate() (line 52) calls lack error handling, so terminal attachment or config dismissal failures fail silently. Add onError callbacks with prefixed logging.
🛠️ Suggested fix
createOrAttach.mutate({
paneId,
tabId,
workspaceId: data.workspace.id,
initialCommands,
+ },
+ {
+ onError: (error) => {
+ console.error(
+ "[terminal/createOrAttach] Failed to attach terminal",
+ error,
+ );
+ },
+ });
- });
if (!initialCommands) {
toast.info("No setup script configured", {
description: "Automate workspace setup with a config.json file",
action: {
label: "Configure",
onClick: () => openConfigModal(data.projectId),
},
onDismiss: () => {
dismissConfigToast.mutate({ projectId: data.projectId });
+ },
+ {
+ onError: (error) => {
+ console.error(
+ "[config/dismissToast] Failed to dismiss config toast",
+ error,
+ );
+ },
+ });
- },📝 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.
| createOrAttach.mutate({ | |
| paneId, | |
| tabId, | |
| workspaceId: data.workspace.id, | |
| initialCommands, | |
| }); | |
| if (!initialCommands) { | |
| toast.info("No setup script configured", { | |
| description: "Automate workspace setup with a config.json file", | |
| action: { | |
| label: "Configure", | |
| onClick: () => openConfigModal(data.projectId), | |
| }, | |
| onDismiss: () => { | |
| dismissConfigToast.mutate({ projectId: data.projectId }); | |
| }, | |
| createOrAttach.mutate( | |
| { | |
| paneId, | |
| tabId, | |
| workspaceId: data.workspace.id, | |
| initialCommands, | |
| }, | |
| { | |
| onError: (error) => { | |
| console.error( | |
| "[terminal/createOrAttach] Failed to attach terminal", | |
| error, | |
| ); | |
| }, | |
| } | |
| ); | |
| if (!initialCommands) { | |
| toast.info("No setup script configured", { | |
| description: "Automate workspace setup with a config.json file", | |
| action: { | |
| label: "Configure", | |
| onClick: () => openConfigModal(data.projectId), | |
| }, | |
| onDismiss: () => { | |
| dismissConfigToast.mutate( | |
| { projectId: data.projectId }, | |
| { | |
| onError: (error) => { | |
| console.error( | |
| "[config/dismissToast] Failed to dismiss config toast", | |
| error, | |
| ); | |
| }, | |
| } | |
| ); | |
| }, |
🤖 Prompt for AI Agents
In `@apps/desktop/src/renderer/react-query/workspaces/useOpenExternalWorktree.ts`
around lines 37 - 53, The createOrAttach.mutate(...) and
dismissConfigToast.mutate(...) calls lack error handling and may fail silently;
update both calls (createOrAttach.mutate and dismissConfigToast.mutate) to pass
an options object with an onError handler that logs the error with a clear
prefix (e.g., "createOrAttach failed:" and "dismissConfigToast failed:"), and
optionally surface a user-facing toast/error notification; ensure the onError
handlers reference the mutation result so failures are visible and diagnosable.
External worktrees created via git CLI won't have .superset directory if it's gitignored. This caused setup commands referencing ./.superset/setup.sh to fail. Now we copy .superset from main repo before loading setup config.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
1 participant
Summary
listDiskWorktreesfunction to discover git worktrees on disk viagit worktree list --porcelaingetUntrackedDiskWorktreestRPC procedure to return worktrees not tracked in the databaseopenDiskWorktreetRPC procedure to import and open a disk worktree as a workspaceTest plan
git worktree add ../test-branch -b test-branchSummary by CodeRabbit