From 7e6b9e0cfff3501f5cd82bfddb44c27c6d62b3fc Mon Sep 17 00:00:00 2001
From: MartinYe1234 <52641447+MartinYe1234@users.noreply.github.com>
Date: Mon, 15 Jun 2026 19:32:29 -0700
Subject: [PATCH] git_ui: Resolve worktree picker remove-from-window clicks by
 path (#59084)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Follow-up to #58996. The "Remove Worktree from Window" button captured
the row index at render time and re-resolved it against the picker's
matches at click time. Matches are rebuilt asynchronously (initial
worktree list load, post-delete refresh, query changes), so the index
could drift between the rendered frame and the click being processed,
closing the workspace of a different worktree than the one the user
clicked.

This changes the click handler to capture the worktree's path — a stable
identity — instead of its index, adds the same `deleting_worktree_paths`
click-time guard the neighboring handlers use, and constructs the button
only when it will actually be shown.

Scoped to the code added in #58996; the same stale-index pattern in the
delete and open-in-new-window handlers is left for a later change.

Release Notes:

- Fixed an issue where the worktree picker's "Remove Worktree from
Window" button could act on the wrong worktree if the list updated at
the moment of the click.
---
 crates/git_ui/src/worktree_picker.rs | 41 ++++++++++++++++------------
 1 file changed, 23 insertions(+), 18 deletions(-)

diff --git a/crates/git_ui/src/worktree_picker.rs b/crates/git_ui/src/worktree_picker.rs
index 312697bf1b..28b5ccd68f 100644
--- a/crates/git_ui/src/worktree_picker.rs
+++ b/crates/git_ui/src/worktree_picker.rs
@@ -702,15 +702,14 @@ impl WorktreePickerDelegate {
 
     fn remove_worktree_from_window(
         &mut self,
-        ix: usize,
+        worktree_path: &Path,
         window: &mut Window,
         cx: &mut Context<Picker<Self>>,
     ) {
-        let Some(WorktreeEntry::Worktree { worktree, .. }) = self.matches.get(ix) else {
+        if self.deleting_worktree_paths.contains(worktree_path) {
             return;
-        };
-        let Some(workspace_to_remove) =
-            self.workspace_for_open_worktree(&worktree.path, window, cx)
+        }
+        let Some(workspace_to_remove) = self.workspace_for_open_worktree(worktree_path, window, cx)
         else {
             return;
         };
@@ -1314,22 +1313,29 @@ impl PickerDelegate for WorktreePickerDelegate {
                                         })),
                                 );
 
-                            let remove_from_window_button = IconButton::new(
-                                ("remove-worktree-from-window", ix),
-                                IconName::Close,
-                            )
-                            .icon_size(IconSize::Small)
-                            .tooltip(Tooltip::text("Remove Worktree from Window"))
-                            .on_click(cx.listener(move |picker, _, window, cx| {
-                                picker.delegate.remove_worktree_from_window(ix, window, cx);
-                            }));
-
                             this.end_slot(
                                 h_flex()
                                     .gap_0p5()
                                     .child(open_in_new_window_button)
                                     .when(can_remove_from_window, |this| {
-                                        this.child(remove_from_window_button)
+                                        let worktree_path = worktree.path.clone();
+                                        this.child(
+                                            IconButton::new(
+                                                ("remove-worktree-from-window", ix),
+                                                IconName::Close,
+                                            )
+                                            .icon_size(IconSize::Small)
+                                            .tooltip(Tooltip::text("Remove Worktree from Window"))
+                                            .on_click(
+                                                cx.listener(move |picker, _, window, cx| {
+                                                    picker.delegate.remove_worktree_from_window(
+                                                        &worktree_path,
+                                                        window,
+                                                        cx,
+                                                    );
+                                                }),
+                                            ),
+                                        )
                                     })
                                     .when(can_delete, |this| this.child(delete_button)),
                             )
@@ -2211,7 +2217,6 @@ mod tests {
         });
         cx.run_until_parked();
 
-        let index = worktree_index(&worktree_picker, &worktree_path, &mut cx);
         worktree_picker.update(&mut cx, |worktree_picker, cx| {
             worktree_picker.picker.update(cx, |picker, _| {
                 assert!(
@@ -2228,7 +2233,7 @@ mod tests {
             worktree_picker.picker.update(cx, |picker, cx| {
                 picker
                     .delegate
-                    .remove_worktree_from_window(index, window, cx);
+                    .remove_worktree_from_window(&worktree_path, window, cx);
             })
         });
         cx.run_until_parked();