fix(desktop): simplify tracked worktree recovery#2721
Closed
Kitenite wants to merge 1 commit into
Closed
Conversation
Contributor
📝 WalkthroughWalkthroughThis pull request introduces a comprehensive worktree path resolution and repair system for the desktop application. Previously, worktree paths were stored directly in the local database. Now, paths are resolved dynamically with fallback repair mechanisms that can detect missing paths, query Git for updated locations, and guide users through recovery workflows when worktrees become unregistered. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as Workspace UI
participant React as React Query<br/>(useRecoverTrackedWorktree)
participant Picker as Directory<br/>Picker
participant TRPC as TRPC Backend<br/>(repairTrackedWorktreePath)
participant Resolver as Worktree Path<br/>Resolver
participant Git as Git/FS Ops<br/>(repairWorktreeRegistration)
participant DB as LocalDB
User->>UI: Opens repair-required worktree
UI->>React: Calls recoverTrackedWorktree()
React->>Picker: Opens directory picker
User->>Picker: Selects new worktree path
Picker-->>React: Returns selectedPath
React->>TRPC: Calls repairTrackedWorktreePath<br/>{workspaceId, selectedPath}
TRPC->>Resolver: Validates workspace + branch
Resolver->>Git: Runs git worktree repair
Git-->>Resolver: Confirms registration updated
Resolver->>Git: Queries Git for worktree path<br/>on expected branch
Git-->>Resolver: Returns verified path
Resolver->>DB: Persists updated path +<br/>pathChanged flag
Resolver-->>TRPC: Returns {path, pathChanged}
TRPC-->>React: Mutation success
React->>React: Invalidate workspaces<br/>& terminal caches
React-->>UI: Show success toast
UI-->>User: Workspace recovered
Estimated code review effort🎯 5 (Critical) | ⏱️ ~105 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
7 issues found across 14 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/lib/trpc/routers/workspaces/procedures/git-status.ts">
<violation number="1" location="apps/desktop/src/lib/trpc/routers/workspaces/procedures/git-status.ts:239">
P2: Use a cross-platform path utility (for example `path.basename`) instead of `split("/")`; this parsing fails for Windows worktree paths.
(Based on your team's feedback about using cross-platform path utilities instead of manual split.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/desktop/src/renderer/react-query/workspaces/useRecoverTrackedWorktree.ts">
<violation number="1" location="apps/desktop/src/renderer/react-query/workspaces/useRecoverTrackedWorktree.ts:43">
P2: Avoid swallowing errors with an empty catch block; log a warning with context so recovery failures remain diagnosable.
(Based on your team's feedback about avoiding empty catch blocks that hide failures.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/page.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/page.tsx:670">
P2: Handle the recovery promise rejection to avoid unhandled promise rejections when directory selection fails.
(Based on your team's feedback about handling async calls with proper await/catch.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/desktop/src/lib/trpc/routers/workspaces/utils/repair-worktree-path.ts">
<violation number="1" location="apps/desktop/src/lib/trpc/routers/workspaces/utils/repair-worktree-path.ts:134">
P1: Do not resolve the tracked worktree when the stored path points to the main repository; this bypasses repair handling and can route workspace actions to the wrong repo.</violation>
</file>
<file name="apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx">
<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx:487">
P2: Handle the `recoverTrackedWorktree` promise rejection instead of discarding it with `void`; a failure in directory selection can surface as an unhandled promise rejection.
(Based on your team's feedback about handling async calls with proper await/catch.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts">
<violation number="1" location="apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts:836">
P2: Include `mainRepoPath`/`worktreePath` in the repair failure log so recovery errors retain actionable context.
(Based on your team's feedback about preserving diagnostic context in errors.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/desktop/src/lib/trpc/routers/terminal/terminal.ts">
<violation number="1" location="apps/desktop/src/lib/trpc/routers/terminal/terminal.ts:100">
P2: Worktree usability checks are skipped when `type === "worktree"` but `worktreeId` is missing, allowing terminal creation with an invalid/missing workspace path.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| }; | ||
| } | ||
|
|
||
| if (existsSync(context.worktree.path)) { |
Contributor
There was a problem hiding this comment.
P1: Do not resolve the tracked worktree when the stored path points to the main repository; this bypasses repair handling and can route workspace actions to the wrong repo.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/lib/trpc/routers/workspaces/utils/repair-worktree-path.ts, line 134:
<comment>Do not resolve the tracked worktree when the stored path points to the main repository; this bypasses repair handling and can route workspace actions to the wrong repo.</comment>
<file context>
@@ -0,0 +1,398 @@
+ };
+ }
+
+ if (existsSync(context.worktree.path)) {
+ return {
+ pathChanged: false,
</file context>
|
|
||
| const worktreeName = worktree.path.split("/").pop() ?? worktree.branch; | ||
| const resolvedPath = await resolveWorktreePathWithRepair(worktree.id); | ||
| const worktreeName = resolvedPath?.split("/").pop() ?? worktree.branch; |
Contributor
There was a problem hiding this comment.
P2: Use a cross-platform path utility (for example path.basename) instead of split("/"); this parsing fails for Windows worktree paths.
(Based on your team's feedback about using cross-platform path utilities instead of manual split.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/lib/trpc/routers/workspaces/procedures/git-status.ts, line 239:
<comment>Use a cross-platform path utility (for example `path.basename`) instead of `split("/")`; this parsing fails for Windows worktree paths.
(Based on your team's feedback about using cross-platform path utilities instead of manual split.) </comment>
<file context>
@@ -217,7 +235,8 @@ export const createGitStatusProcedures = () => {
- const worktreeName = worktree.path.split("/").pop() ?? worktree.branch;
+ const resolvedPath = await resolveWorktreePathWithRepair(worktree.id);
+ const worktreeName = resolvedPath?.split("/").pop() ?? worktree.branch;
const branchName = worktree.branch;
</file context>
| workspaceId, | ||
| selectedPath: result.path, | ||
| }); | ||
| } catch { |
Contributor
There was a problem hiding this comment.
P2: Avoid swallowing errors with an empty catch block; log a warning with context so recovery failures remain diagnosable.
(Based on your team's feedback about avoiding empty catch blocks that hide failures.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/react-query/workspaces/useRecoverTrackedWorktree.ts, line 43:
<comment>Avoid swallowing errors with an empty catch block; log a warning with context so recovery failures remain diagnosable.
(Based on your team's feedback about avoiding empty catch blocks that hide failures.) </comment>
<file context>
@@ -0,0 +1,52 @@
+ workspaceId,
+ selectedPath: result.path,
+ });
+ } catch {
+ // Mutation onError already surfaces the failure to the user.
+ }
</file context>
| <div className="mt-4 flex gap-2"> | ||
| <Button | ||
| size="sm" | ||
| onClick={() => void recoverTrackedWorktree()} |
Contributor
There was a problem hiding this comment.
P2: Handle the recovery promise rejection to avoid unhandled promise rejections when directory selection fails.
(Based on your team's feedback about handling async calls with proper await/catch.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/page.tsx, line 670:
<comment>Handle the recovery promise rejection to avoid unhandled promise rejections when directory selection fails.
(Based on your team's feedback about handling async calls with proper await/catch.) </comment>
<file context>
@@ -632,6 +650,43 @@ function WorkspacePage() {
+ <div className="mt-4 flex gap-2">
+ <Button
+ size="sm"
+ onClick={() => void recoverTrackedWorktree()}
+ disabled={isRecoverTrackedWorktreePending}
+ >
</file context>
| isUnread={isUnread} | ||
| workspaceStatus={workspaceStatus} | ||
| sections={sections} | ||
| onRecoverWorktree={() => void recoverTrackedWorktree()} |
Contributor
There was a problem hiding this comment.
P2: Handle the recoverTrackedWorktree promise rejection instead of discarding it with void; a failure in directory selection can surface as an unhandled promise rejection.
(Based on your team's feedback about handling async calls with proper await/catch.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx, line 487:
<comment>Handle the `recoverTrackedWorktree` promise rejection instead of discarding it with `void`; a failure in directory selection can surface as an unhandled promise rejection.
(Based on your team's feedback about handling async calls with proper await/catch.) </comment>
<file context>
@@ -440,9 +480,11 @@ export function WorkspaceListItem({
isUnread={isUnread}
workspaceStatus={workspaceStatus}
sections={sections}
+ onRecoverWorktree={() => void recoverTrackedWorktree()}
onRename={rename.startRename}
onOpenInFinder={handleOpenInFinder}
</file context>
Suggested change
| onRecoverWorktree={() => void recoverTrackedWorktree()} | |
| onRecoverWorktree={() => { | |
| void recoverTrackedWorktree().catch((error) => { | |
| toast.error(`Failed to recover worktree: ${error.message}`); | |
| }); | |
| }} |
| const git = await getSimpleGitWithShellPath(mainRepoPath); | ||
| await git.raw(["worktree", "repair", worktreePath]); | ||
| } catch (error) { | ||
| console.error(`Failed to repair worktree registration: ${error}`); |
Contributor
There was a problem hiding this comment.
P2: Include mainRepoPath/worktreePath in the repair failure log so recovery errors retain actionable context.
(Based on your team's feedback about preserving diagnostic context in errors.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts, line 836:
<comment>Include `mainRepoPath`/`worktreePath` in the repair failure log so recovery errors retain actionable context.
(Based on your team's feedback about preserving diagnostic context in errors.) </comment>
<file context>
@@ -822,6 +822,22 @@ export async function getBranchWorktreePath({
+ const git = await getSimpleGitWithShellPath(mainRepoPath);
+ await git.raw(["worktree", "repair", worktreePath]);
+ } catch (error) {
+ console.error(`Failed to repair worktree registration: ${error}`);
+ throw error;
+ }
</file context>
Suggested change
| console.error(`Failed to repair worktree registration: ${error}`); | |
| console.error(`Failed to repair worktree registration for repo ${mainRepoPath} and worktree ${worktreePath}: ${error}`); |
Comment on lines
+100
to
107
| if (workspace?.type === "worktree" && workspace.worktreeId) { | ||
| const resolved = await resolveWorktreePathOrThrowWithMetadata( | ||
| workspace.worktreeId, | ||
| ); | ||
| workspacePath = resolved.path ?? undefined; | ||
| pathChanged = resolved.pathChanged; | ||
| assertWorkspaceUsable(workspaceId, workspacePath); | ||
| } |
Contributor
There was a problem hiding this comment.
P2: Worktree usability checks are skipped when type === "worktree" but worktreeId is missing, allowing terminal creation with an invalid/missing workspace path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/lib/trpc/routers/terminal/terminal.ts, line 100:
<comment>Worktree usability checks are skipped when `type === "worktree"` but `worktreeId` is missing, allowing terminal creation with an invalid/missing workspace path.</comment>
<file context>
@@ -86,9 +90,19 @@ export const createTerminalRouter = () => {
+ } = getWorkspaceTerminalContext(workspaceId);
+ let workspacePath = baseWorkspacePath;
+ let pathChanged = false;
+ if (workspace?.type === "worktree" && workspace.worktreeId) {
+ const resolved = await resolveWorktreePathOrThrowWithMetadata(
+ workspace.worktreeId,
</file context>
Suggested change
| if (workspace?.type === "worktree" && workspace.worktreeId) { | |
| const resolved = await resolveWorktreePathOrThrowWithMetadata( | |
| workspace.worktreeId, | |
| ); | |
| workspacePath = resolved.path ?? undefined; | |
| pathChanged = resolved.pathChanged; | |
| assertWorkspaceUsable(workspaceId, workspacePath); | |
| } | |
| if (workspace?.type === "worktree") { | |
| if (workspace.worktreeId) { | |
| const resolved = await resolveWorktreePathOrThrowWithMetadata( | |
| workspace.worktreeId, | |
| ); | |
| workspacePath = resolved.path ?? undefined; | |
| pathChanged = resolved.pathChanged; | |
| } | |
| assertWorkspaceUsable(workspaceId, workspacePath); | |
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/repair-worktree-path.ts`:
- Around line 112-120: The command string built by
getTrackedWorktreeRepairCommand/getTrackedWorktreeRepairMessage currently
interpolates mainRepoPath raw into double quotes which can be broken by
characters like $, `, ", etc.; change these functions to produce a POSIX-safe
quoted command by shell-escaping mainRepoPath (e.g., wrap in single quotes and
escape any embedded single quotes) and also quote the "<new-path>" placeholder
(e.g., '<new-path>') so the full returned command is safe to paste into a POSIX
shell; implement the escaping in a small helper used by
getTrackedWorktreeRepairCommand and update getTrackedWorktreeRepairMessage to
reference the escaped command.
- Around line 134-141: The early-return fast path that returns resolved when
existsSync(context.worktree.path) is bypassing the main-repo guard and skips
calling persistResolvedTrackedWorktreePath; update the fast path in
repairWorktreePath (the block that checks existsSync(context.worktree.path)) to
run the same "main repo" check used elsewhere (ensure the candidate path is not
equal to project.mainRepoPath or whatever main-repo sentinel your code uses)
and, if allowed, call persistResolvedTrackedWorktreePath(...) before returning
so tracked worktree records are updated; if the path matches the main repo,
treat it as not resolved and fall through to the existing recovery logic.
- Around line 326-364: The current flow calls getGitRoot/getCurrentBranch before
repairing worktree metadata, causing "not a git repository" errors for moved
worktrees; change the order so you first resolve and validate the path exists
(use existsSync and realpathSync inside isMainRepoPath checks against
context.mainRepoPath and input.selectedPath), refuse if it's the main repo via
isMainRepoPath(context, candidatePath) after realpath resolution, then call
repairWorktreeRegistration({ mainRepoPath: context.mainRepoPath, worktreePath:
candidatePath }), and only after repair re-run
getGitRoot/getBranchWorktreePath/getCurrentBranch to verify the branch matches
context.worktree.branch. Ensure you still throw the same TRPCError
codes/messages where appropriate but perform filesystem checks and repair before
querying Git state.
In
`@apps/desktop/src/renderer/react-query/workspaces/useRecoverTrackedWorktree.ts`:
- Around line 29-45: The await call to selectDirectory.mutateAsync in
recoverTrackedWorktree is outside the try/catch and can cause unhandled promise
rejections; move the selectDirectory.mutateAsync call into the existing try
block (or add a surrounding try/catch) so any picker errors are caught, and in
the catch handle or surface the error consistently (e.g., show user feedback or
log via the same mechanism used by repairTrackedWorktree.onError) to avoid
unhandled rejections when callers call void recoverTrackedWorktree().
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 564a7dbd-4b72-4f92-b82a-bd5da4a63413
📒 Files selected for processing (14)
apps/desktop/src/lib/trpc/routers/terminal/terminal.tsapps/desktop/src/lib/trpc/routers/workspaces/procedures/git-status.tsapps/desktop/src/lib/trpc/routers/workspaces/procedures/query.tsapps/desktop/src/lib/trpc/routers/workspaces/procedures/repair.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/git.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/repair-worktree-path.tsapps/desktop/src/lib/trpc/routers/workspaces/workspaces.tsapps/desktop/src/renderer/react-query/workspaces/index.tsapps/desktop/src/renderer/react-query/workspaces/useRecoverTrackedWorktree.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/page.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceList/WorkspaceList.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceContextMenu.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/types.ts
Comment on lines
+112
to
+120
| export function getTrackedWorktreeRepairCommand(mainRepoPath: string): string { | ||
| return `git -C "${mainRepoPath}" worktree repair <new-path>`; | ||
| } | ||
|
|
||
| export function getTrackedWorktreeRepairMessage(input: { | ||
| branch: string; | ||
| mainRepoPath: string; | ||
| }): string { | ||
| return `Worktree branch "${input.branch}" is missing at its tracked path. Select the moved worktree folder and Superset will repair it, or run ${getTrackedWorktreeRepairCommand(input.mainRepoPath)} manually.`; |
Contributor
There was a problem hiding this comment.
Shell-escape the manual repair command.
mainRepoPath is interpolated inside double quotes, so $(), backticks, $VAR, and embedded " characters can still alter or break the pasted command. Build this string with POSIX-safe shell quoting instead of raw interpolation, and quote the <new-path> placeholder too.
🔒 Suggested fix
+function shellQuote(value: string): string {
+ return `'${value.replace(/'/g, "'\\''")}'`;
+}
+
export function getTrackedWorktreeRepairCommand(mainRepoPath: string): string {
- return `git -C "${mainRepoPath}" worktree repair <new-path>`;
+ return `git -C ${shellQuote(mainRepoPath)} worktree repair '<new-path>'`;
}Based on learnings, Windows is not supported in the apps/desktop desktop app, so POSIX shell quoting is the relevant target here.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export function getTrackedWorktreeRepairCommand(mainRepoPath: string): string { | |
| return `git -C "${mainRepoPath}" worktree repair <new-path>`; | |
| } | |
| export function getTrackedWorktreeRepairMessage(input: { | |
| branch: string; | |
| mainRepoPath: string; | |
| }): string { | |
| return `Worktree branch "${input.branch}" is missing at its tracked path. Select the moved worktree folder and Superset will repair it, or run ${getTrackedWorktreeRepairCommand(input.mainRepoPath)} manually.`; | |
| function shellQuote(value: string): string { | |
| return `'${value.replace(/'/g, "'\\''")}'`; | |
| } | |
| export function getTrackedWorktreeRepairCommand(mainRepoPath: string): string { | |
| return `git -C ${shellQuote(mainRepoPath)} worktree repair '<new-path>'`; | |
| } | |
| export function getTrackedWorktreeRepairMessage(input: { | |
| branch: string; | |
| mainRepoPath: string; | |
| }): string { | |
| return `Worktree branch "${input.branch}" is missing at its tracked path. Select the moved worktree folder and Superset will repair it, or run ${getTrackedWorktreeRepairCommand(input.mainRepoPath)} manually.`; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/repair-worktree-path.ts`
around lines 112 - 120, The command string built by
getTrackedWorktreeRepairCommand/getTrackedWorktreeRepairMessage currently
interpolates mainRepoPath raw into double quotes which can be broken by
characters like $, `, ", etc.; change these functions to produce a POSIX-safe
quoted command by shell-escaping mainRepoPath (e.g., wrap in single quotes and
escape any embedded single quotes) and also quote the "<new-path>" placeholder
(e.g., '<new-path>') so the full returned command is safe to paste into a POSIX
shell; implement the escaping in a small helper used by
getTrackedWorktreeRepairCommand and update getTrackedWorktreeRepairMessage to
reference the escaped command.
Comment on lines
+134
to
+141
| if (existsSync(context.worktree.path)) { | ||
| return { | ||
| pathChanged: false, | ||
| resolution: { | ||
| status: "resolved", | ||
| path: context.worktree.path, | ||
| }, | ||
| }; |
Contributor
There was a problem hiding this comment.
Run the stored-path fast path through the same main-repo guard.
This early return bypasses persistResolvedTrackedWorktreePath(). If a tracked worktree record ever points at project.mainRepoPath, resolve*() still reports it as "resolved", and the new terminal/git-status callers keep operating on the main checkout instead of surfacing recovery.
🐛 Suggested fix
if (existsSync(context.worktree.path)) {
- return {
- pathChanged: false,
- resolution: {
- status: "resolved",
- path: context.worktree.path,
- },
- };
+ return persistResolvedTrackedWorktreePath({
+ context,
+ resolvedPath: context.worktree.path,
+ });
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/repair-worktree-path.ts`
around lines 134 - 141, The early-return fast path that returns resolved when
existsSync(context.worktree.path) is bypassing the main-repo guard and skips
calling persistResolvedTrackedWorktreePath; update the fast path in
repairWorktreePath (the block that checks existsSync(context.worktree.path)) to
run the same "main repo" check used elsewhere (ensure the candidate path is not
equal to project.mainRepoPath or whatever main-repo sentinel your code uses)
and, if allowed, call persistResolvedTrackedWorktreePath(...) before returning
so tracked worktree records are updated; if the path matches the main repo,
treat it as not resolved and fall through to the existing recovery logic.
Comment on lines
+326
to
+364
| let candidatePath: string; | ||
| try { | ||
| candidatePath = await getGitRoot(input.selectedPath); | ||
| } catch (error) { | ||
| throw new TRPCError({ | ||
| code: "BAD_REQUEST", | ||
| message: | ||
| error instanceof Error | ||
| ? error.message | ||
| : "Selected path is not a Git worktree", | ||
| }); | ||
| } | ||
|
|
||
| if (!existsSync(candidatePath)) { | ||
| throw new TRPCError({ | ||
| code: "BAD_REQUEST", | ||
| message: "Selected worktree path does not exist on disk", | ||
| }); | ||
| } | ||
|
|
||
| if (isMainRepoPath(context, candidatePath)) { | ||
| throw new TRPCError({ | ||
| code: "BAD_REQUEST", | ||
| message: "Select the moved worktree folder, not the main repository", | ||
| }); | ||
| } | ||
|
|
||
| const currentBranch = await getCurrentBranch(candidatePath); | ||
| if (currentBranch !== context.worktree.branch) { | ||
| throw new TRPCError({ | ||
| code: "PRECONDITION_FAILED", | ||
| message: `Selected folder is on branch "${currentBranch ?? "detached"}", expected "${context.worktree.branch}"`, | ||
| }); | ||
| } | ||
|
|
||
| await repairWorktreeRegistration({ | ||
| mainRepoPath: context.mainRepoPath, | ||
| worktreePath: candidatePath, | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
After manually moving a linked Git worktree without using git worktree move, do commands like git -C rev-parse --show-toplevelorgit -C branch --show-currentfail untilgit worktree repair is run?
💡 Result:
No, commands like git -C rev-parse --show-toplevel or git -C branch --show-current do not fail after manually moving a linked Git worktree, provided you correctly update the gitdir file in the main repository's .git/worktrees// directory to point to the new path. The official Git documentation explicitly states how to do this manually: "If you manually move a linked worktree, you need to update the gitdir file in the entry’s directory. For example, if a linked worktree is moved to /newpath/test-next and its .git file points to /path/main/.git/worktrees/test-next, then update /path/main/.git/worktrees/test-next/gitdir to reference /newpath/test-next instead." It recommends git worktree repair as a better automated alternative, but manual update works without repair. The worktree's own .git file (gitlink) does not need changing because it points to the fixed worktree subdirectory in the main repo, not the worktree path itself. Failures in rev-parse --show-toplevel occur in other scenarios like cross-OS path issues (e.g., WSL/Windows), but not due to manual moves when properly updated.
Citations:
- 1: https://git-scm.com/docs/git-worktree
- 2: https://git-scm.com/docs/git-worktree.html
- 3: https://man.cx/git-worktree(1)
- 4: https://stackoverflow.com/questions/76838262/git-rev-parse-show-toplevel-not-working-for-worktree-within-wsl
🏁 Script executed:
# First, let's examine the file in question
cd apps/desktop/src/lib/trpc/routers/workspaces/utils && wc -l repair-worktree-path.tsRepository: superset-sh/superset
Length of output: 91
🏁 Script executed:
# Get the full context of the function to understand the scenario
cd apps/desktop/src/lib/trpc/routers/workspaces/utils && cat -n repair-worktree-path.ts | head -80Repository: superset-sh/superset
Length of output: 2295
🏁 Script executed:
# Look at the functions being called: getGitRoot, getCurrentBranch
rg -n "export.*getGitRoot|export.*getCurrentBranch" --type ts --type tsxRepository: superset-sh/superset
Length of output: 91
🏁 Script executed:
# Look at the repair function being called
rg -n "export.*repairWorktreeRegistration" --type ts --type tsxRepository: superset-sh/superset
Length of output: 91
🏁 Script executed:
# Check the git.ts file to understand what these functions do
rg -n "export.*getGitRoot|export.*getCurrentBranch|export.*repairWorktreeRegistration" --type tsRepository: superset-sh/superset
Length of output: 407
🏁 Script executed:
# Find the git.ts file
fd -n "git.ts" apps/desktop/src/lib/trpc/routers/workspaces/utilsRepository: superset-sh/superset
Length of output: 294
🏁 Script executed:
# Let's look at the full repair-worktree-path.ts file to understand the context better
cd apps/desktop/src/lib/trpc/routers/workspaces/utils && cat -n repair-worktree-path.tsRepository: superset-sh/superset
Length of output: 12568
🏁 Script executed:
# Get the implementation of getGitRoot
sed -n '699,750p' apps/desktop/src/lib/trpc/routers/workspaces/utils/git.tsRepository: superset-sh/superset
Length of output: 1563
🏁 Script executed:
# Get the implementation of getCurrentBranch
sed -n '1329,1380p' apps/desktop/src/lib/trpc/routers/workspaces/utils/git.tsRepository: superset-sh/superset
Length of output: 1478
🏁 Script executed:
# Get the implementation of repairWorktreeRegistration
sed -n '825,900p' apps/desktop/src/lib/trpc/routers/workspaces/utils/git.tsRepository: superset-sh/superset
Length of output: 2297
🏁 Script executed:
# Search for any tests or comments about worktree repair scenarios
rg -i "manual.*move|moved.*worktree" apps/desktop/src/lib/trpc/routers/workspaces --type ts -A 3 -B 3Repository: superset-sh/superset
Length of output: 4601
🏁 Script executed:
# Check what getBranchWorktreePath does (it's called AFTER repair)
sed -n '500,600p' apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts | head -100Repository: superset-sh/superset
Length of output: 3222
🏁 Script executed:
# Search for getBranchWorktreePath implementation
rg -n "export.*getBranchWorktreePath|function getBranchWorktreePath" apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts -A 15Repository: superset-sh/superset
Length of output: 582
🏁 Script executed:
# Search for any error handling or comments about git worktree repair behavior
rg "git worktree repair|gitdir" apps/desktop/src --type ts -B 2 -A 2Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Check the simple-git wrapper to see what errors getGitRoot would throw
rg -n "getSimpleGitWithShellPath|NotGitRepoError" apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts | head -20Repository: superset-sh/superset
Length of output: 1269
🏁 Script executed:
# Look at the full context around the repairTrackedWorktreePath function
sed -n '311,398p' apps/desktop/src/lib/trpc/routers/workspaces/utils/repair-worktree-path.ts | cat -nRepository: superset-sh/superset
Length of output: 2772
🏁 Script executed:
# Look at what happens when a git command fails in this context
sed -n '699,720p' apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts | cat -nRepository: superset-sh/superset
Length of output: 886
Reorder validations to repair Git metadata before querying worktree state.
For a manually moved linked worktree without using git worktree move, the .git/worktrees/<id>/gitdir file remains pointed at the old path. When getGitRoot and getCurrentBranch run from the new path before repair, they fail with "not a git repository" errors, causing the mutation to reject the exact folder it should recover.
Perform the filesystem checks first (existsSync, isMainRepoPath using realpathSync), then run repairWorktreeRegistration, and only then re-query the worktree state with getBranchWorktreePath and branch verification.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/repair-worktree-path.ts`
around lines 326 - 364, The current flow calls getGitRoot/getCurrentBranch
before repairing worktree metadata, causing "not a git repository" errors for
moved worktrees; change the order so you first resolve and validate the path
exists (use existsSync and realpathSync inside isMainRepoPath checks against
context.mainRepoPath and input.selectedPath), refuse if it's the main repo via
isMainRepoPath(context, candidatePath) after realpath resolution, then call
repairWorktreeRegistration({ mainRepoPath: context.mainRepoPath, worktreePath:
candidatePath }), and only after repair re-run
getGitRoot/getBranchWorktreePath/getCurrentBranch to verify the branch matches
context.worktree.branch. Ensure you still throw the same TRPCError
codes/messages where appropriate but perform filesystem checks and repair before
querying Git state.
Comment on lines
+29
to
+45
| const recoverTrackedWorktree = async () => { | ||
| const result = await selectDirectory.mutateAsync({ | ||
| title: "Select moved worktree folder", | ||
| defaultPath: defaultPath ?? undefined, | ||
| }); | ||
| if (result.canceled || !result.path) { | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| await repairTrackedWorktree.mutateAsync({ | ||
| workspaceId, | ||
| selectedPath: result.path, | ||
| }); | ||
| } catch { | ||
| // Mutation onError already surfaces the failure to the user. | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that selectDirectory is awaited outside the try/catch
# and callers use fire-and-forget invocation.
rg -n -C2 'selectDirectory\.mutateAsync|repairTrackedWorktree\.mutateAsync|void recoverTrackedWorktree\(\)' \
apps/desktop/src/renderer/react-query/workspaces/useRecoverTrackedWorktree.ts \
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx \
apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/\$workspaceId/page.tsxRepository: superset-sh/superset
Length of output: 2546
🏁 Script executed:
cat -n apps/desktop/src/renderer/react-query/workspaces/useRecoverTrackedWorktree.tsRepository: superset-sh/superset
Length of output: 1793
Wrap folder-picker mutation in error handling to prevent unhandled rejections.
Line 30 awaits selectDirectory.mutateAsync() outside the try/catch block. Since callers invoke this function with void recoverTrackedWorktree(), any picker error becomes an unhandled promise rejection with no user feedback. The repair mutation has onError feedback, but the picker does not.
Suggested fix
const recoverTrackedWorktree = async () => {
- const result = await selectDirectory.mutateAsync({
- title: "Select moved worktree folder",
- defaultPath: defaultPath ?? undefined,
- });
- if (result.canceled || !result.path) {
- return;
- }
+ let result: { canceled: boolean; path?: string };
+ try {
+ result = await selectDirectory.mutateAsync({
+ title: "Select moved worktree folder",
+ defaultPath: defaultPath ?? undefined,
+ });
+ } catch (error) {
+ toast.error(
+ `Failed to open folder picker: ${
+ error instanceof Error ? error.message : "Unknown error"
+ }`,
+ );
+ return;
+ }
+ if (result.canceled || !result.path) return;
try {
await repairTrackedWorktree.mutateAsync({
workspaceId,
selectedPath: result.path,
});
} catch {
// Mutation onError already surfaces the failure to the user.
}
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/react-query/workspaces/useRecoverTrackedWorktree.ts`
around lines 29 - 45, The await call to selectDirectory.mutateAsync in
recoverTrackedWorktree is outside the try/catch and can cause unhandled promise
rejections; move the selectDirectory.mutateAsync call into the existing try
block (or add a surrounding try/catch) so any picker errors are caught, and in
the catch handle or surface the error consistently (e.g., show user feedback or
log via the same mechanism used by repairTrackedWorktree.onError) to avoid
unhandled rejections when callers call void recoverTrackedWorktree().
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
origin/mainas a compact implementationrepairTrackedWorktreePathmutation plus sidebar/workspace recovery actions that prompt for the moved folderValidation
bun run --filter @superset/desktop typecheckbunx biome check apps/desktop/src/lib/trpc/routers/workspaces/procedures/query.ts apps/desktop/src/lib/trpc/routers/terminal/terminal.ts apps/desktop/src/lib/trpc/routers/workspaces/procedures/git-status.ts apps/desktop/src/lib/trpc/routers/workspaces/procedures/repair.ts apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts apps/desktop/src/lib/trpc/routers/workspaces/utils/repair-worktree-path.ts apps/desktop/src/renderer/react-query/workspaces/useRecoverTrackedWorktree.ts apps/desktop/src/renderer/react-query/workspaces/index.ts apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/types.ts apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceList/WorkspaceList.tsx apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceContextMenu.tsx apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx 'apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/page.tsx'Notes
Summary by cubic
Adds an explicit tracked worktree recovery flow for Desktop. Detects moved worktrees, guides users to repair via
git worktree repair, and prevents terminal/git flows from using stale paths.New Features
workspaces/utils/repair-worktree-path) that inspect Git, auto-normalize when Git knows the new path, or mark “repair required” with a helpful message and command.workspaces.repairTrackedWorktreePath; query outputs now includeexistsOnDisk,repairState,repairMessage, andrepairCommand.useRecoverTrackedWorktreehook.pathChangedwhen normalization occurs.Bug Fixes
Written for commit d51a007. Summary will update on new commits.
Summary by CodeRabbit