Delete orphaned worktree when removing a thread#23
Merged
juliusmarminge merged 6 commits intomainfrom Feb 13, 2026
Merged
Conversation
- Prompt to remove the worktree when a deleted thread is its only reference - Best-effort cleanup via `api.git.removeWorktree` after thread deletion - Add `getOrphanedWorktreePathForThread` helper with unit tests
WalkthroughAdds utilities to detect orphaned Git worktree paths and format them, integrates a desktop confirmation dialog (IPC, preload bridge, main handler), updates renderer native API, uses confirmation and best-effort worktree removal when deleting threads, and adds tests for utilities and the dialog. Changes
Sequence Diagram(s)sequenceDiagram
participant Renderer as Renderer (Sidebar)
participant Utils as Worktree Utils
participant Native as NativeApi (wsNativeApi)
participant Preload as Preload (desktopBridge)
participant Main as Main (IPC handler)
participant Dialog as Electron Dialog
participant Git as Git API
Renderer->>Utils: getOrphanedWorktreePathForThread(threads, threadId)
Utils-->>Renderer: orphanedPath (string|null)
alt orphanedPath exists
Renderer->>Native: dialogs.confirm("Delete worktree?")
Native->>Preload: window.desktopBridge.confirm(message)
Preload->>Main: IPC "desktop:confirm" with message
Main->>Dialog: showDesktopConfirmDialog(message, ownerWindow)
Dialog-->>Main: userChoice (Cancel|Confirm)
Main-->>Preload: boolean result
Preload-->>Native: boolean result
Native-->>Renderer: confirmed?
alt confirmed
Renderer->>Git: git.removeWorktree({cwd: projectCwd, path: orphanedPath})
Git-->>Renderer: success|error (ignored)
end
end
Renderer->>Renderer: delete thread (always proceeds)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
No actionable comments were generated in the recent review. 🎉 Comment |
Contributor
Prompt to delete an orphaned worktree when a thread is deleted and add
|
![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/web/src/components/Sidebar.tsx`:
- Around line 305-312: The current call to api.git.removeWorktree uses
orphanedWorktreePath as a fallback for cwd which is invalid because git worktree
remove expects a repository root or another worktree; change the logic so you
only call api.git.removeWorktree when threadProject is present and use
threadProject.cwd as the cwd, and if threadProject is undefined either skip the
removal attempt or log a warning (e.g., use console.warn/processLogger.warn)
instead of passing orphanedWorktreePath as cwd; keep the existing try/catch
around the removal to ensure deletion remains best-effort.
🧹 Nitpick comments (2)
apps/web/src/worktreeCleanup.test.ts (1)
48-52: Consider adding a test for whitespace normalization.Since
normalizeWorktreePathtrims input, a test verifying that paths differing only by whitespace are treated as equal would strengthen coverage.🧪 Optional test case
+ it("treats whitespace-padded paths as equal", () => { + const threads = [ + makeThread({ id: "thread-1", worktreePath: "/tmp/repo/worktrees/feature-a" }), + makeThread({ id: "thread-2", worktreePath: " /tmp/repo/worktrees/feature-a " }), + ]; + const result = getOrphanedWorktreePathForThread(threads, "thread-1"); + expect(result).toBeNull(); + });apps/web/src/components/Sidebar.tsx (1)
266-277: Blocking UI withwindow.confirmduring an async handler.Using
window.confirmis functional but blocks the main thread and provides a dated UX. This is acceptable for an MVP but consider replacing with a non-blocking modal in the future for better user experience.
Greptile OverviewGreptile SummaryThis PR adds automatic cleanup of orphaned git worktrees when deleting threads. When a thread is the only reference to a worktree, the user is prompted to delete the worktree along with the thread. The implementation adds a Key changes:
Issues found:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Sidebar
participant getOrphanedWorktreePathForThread
participant API
participant Git
User->>Sidebar: Right-click thread, select Delete
Sidebar->>Sidebar: Find thread in state.threads
Sidebar->>Sidebar: Find threadProject in state.projects
Sidebar->>getOrphanedWorktreePathForThread: Check if worktree is orphaned
getOrphanedWorktreePathForThread->>getOrphanedWorktreePathForThread: Find target thread
getOrphanedWorktreePathForThread->>getOrphanedWorktreePathForThread: Check if worktreePath exists
getOrphanedWorktreePathForThread->>getOrphanedWorktreePathForThread: Check if other threads share same worktreePath
getOrphanedWorktreePathForThread-->>Sidebar: Return orphanedWorktreePath or null
alt orphanedWorktreePath exists
Sidebar->>User: Show confirmation dialog
User-->>Sidebar: Confirm or cancel
end
Sidebar->>API: providers.stopSession (if session active)
Sidebar->>API: terminal.close (if terminal open)
Sidebar->>Sidebar: dispatch DELETE_THREAD
alt shouldDeleteWorktree is true
Sidebar->>API: git.removeWorktree(cwd, path)
API->>Git: Execute git worktree remove
Git-->>API: Success or error
Note over Sidebar: Errors are swallowed (best-effort)
end
Last reviewed commit: 6d9a467 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
- add `dialogs.confirm` to native IPC/contracts and desktop preload/main bridge - switch sidebar worktree deletion prompt from `window.confirm` to native API - add `showDesktopConfirmDialog` helper with unit tests for dialog behavior
Co-authored-by: codex <codex@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/desktop/src/main.ts`:
- Around line 147-155: Wrap the ipcMain handler for CONFIRM_CHANNEL so any
rejection from showDesktopConfirmDialog is caught and results in a safe false
return; specifically, update the ipcMain.handle callback (which currently checks
message type and calls showDesktopConfirmDialog with owner resolved via
BrowserWindow.getFocusedWindow() ?? mainWindow) to use try/catch around the
await call and return false on error, ensuring non-string messages still return
false as before.
In `@apps/web/src/wsNativeApi.ts`:
- Around line 63-68: The confirm method currently forwards to
window.desktopBridge.confirm but will reject the returned promise if the IPC
call fails; wrap the desktopBridge.confirm call in a try/catch inside the
confirm async function (the confirm export) and on error fallback to calling
window.confirm(message) so callers always get a resolved boolean (or fallback
result) instead of a propagated rejection.
Comment on lines
+147
to
+155
| ipcMain.removeHandler(CONFIRM_CHANNEL); | ||
| ipcMain.handle(CONFIRM_CHANNEL, async (_event, message: unknown) => { | ||
| if (typeof message !== "string") { | ||
| return false; | ||
| } | ||
|
|
||
| const owner = BrowserWindow.getFocusedWindow() ?? mainWindow; | ||
| return showDesktopConfirmDialog(message, owner); | ||
| }); |
There was a problem hiding this comment.
Guard confirm IPC against dialog failures.
If showDesktopConfirmDialog rejects (e.g., owner window closed), the IPC call rejects and can surface as a UI error. Consider returning false on failure to keep the flow best-effort.
🔧 Suggested change
ipcMain.handle(CONFIRM_CHANNEL, async (_event, message: unknown) => {
if (typeof message !== "string") {
return false;
}
const owner = BrowserWindow.getFocusedWindow() ?? mainWindow;
- return showDesktopConfirmDialog(message, owner);
+ try {
+ return await showDesktopConfirmDialog(message, owner);
+ } catch {
+ return false;
+ }
});🤖 Prompt for AI Agents
In `@apps/desktop/src/main.ts` around lines 147 - 155, Wrap the ipcMain handler
for CONFIRM_CHANNEL so any rejection from showDesktopConfirmDialog is caught and
results in a safe false return; specifically, update the ipcMain.handle callback
(which currently checks message type and calls showDesktopConfirmDialog with
owner resolved via BrowserWindow.getFocusedWindow() ?? mainWindow) to use
try/catch around the await call and return false on error, ensuring non-string
messages still return false as before.
Comment on lines
+63
to
+68
| confirm: async (message) => { | ||
| if (window.desktopBridge) { | ||
| return window.desktopBridge.confirm(message); | ||
| } | ||
| return window.confirm(message); | ||
| }, |
There was a problem hiding this comment.
Fallback if the desktop confirm IPC rejects.
If the IPC call fails, callers will see a rejected promise. Consider a try/catch fallback to keep confirmation best-effort.
🔧 Suggested change
confirm: async (message) => {
if (window.desktopBridge) {
- return window.desktopBridge.confirm(message);
+ try {
+ return await window.desktopBridge.confirm(message);
+ } catch {
+ // fall through to browser confirm
+ }
}
return window.confirm(message);
},📝 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
| confirm: async (message) => { | |
| if (window.desktopBridge) { | |
| return window.desktopBridge.confirm(message); | |
| } | |
| return window.confirm(message); | |
| }, | |
| confirm: async (message) => { | |
| if (window.desktopBridge) { | |
| try { | |
| return await window.desktopBridge.confirm(message); | |
| } catch { | |
| // fall through to browser confirm | |
| } | |
| } | |
| return window.confirm(message); | |
| }, |
🤖 Prompt for AI Agents
In `@apps/web/src/wsNativeApi.ts` around lines 63 - 68, The confirm method
currently forwards to window.desktopBridge.confirm but will reject the returned
promise if the IPC call fails; wrap the desktopBridge.confirm call in a
try/catch inside the confirm async function (the confirm export) and on error
fallback to calling window.confirm(message) so callers always get a resolved
boolean (or fallback result) instead of a propagated rejection.
Co-authored-by: codex <codex@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/web/src/components/Sidebar.tsx`:
- Around line 268-277: The confirmation dialog is shown even when deletion
cannot proceed because threadProject may be undefined; change the logic around
shouldDeleteWorktree so the api.dialogs.confirm prompt is only shown when
orphanedWorktreePath is non-null AND threadProject (or the project/worker that
indicates the worktree can be deleted) is defined/owned; update the condition
that computes shouldDeleteWorktree (referencing shouldDeleteWorktree,
orphanedWorktreePath, threadProject, and api.dialogs.confirm) so the confirm
dialog is skipped when threadProject is undefined, and ensure the subsequent
deletion branch still checks the same threadProject existence before calling the
worktree delete code.
- Format orphaned worktree paths to display only the final segment - Apply formatting in Sidebar delete prompt for cleaner UX - Add tests for Unix, Windows, trailing slash, and fallback cases
- Require both orphaned worktree path and thread project before prompting delete - Prevents delete confirmation when project metadata is unavailable
DavidIlie
added a commit
to DavidIlie/t3code
that referenced
this pull request
Mar 13, 2026
Mark commits pingdotgg#23-41 as integrated.
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
getOrphanedWorktreePathForThreadhelper to detect whether a worktree is shared by other threads.Testing
apps/web/src/worktreeCleanup.test.tswith 5 Vitest cases for orphaned worktree detection behavior.Summary by CodeRabbit
New Features
Tests