Fix zed irresponsive on symlinked directory events outside the editor (#50746)

prayansh_chhablani created

Closes #48729, closes #27263, closes #45954

This PR aims to make zed responsive on symlinked directory events
outside the editor.

Before you mark this PR as ready for review, make sure that you have:
- [ ] Added a solid test coverage and/or screenshots from doing manual
testing
- [x] Done a self-review taking into account security and performance
aspects
- [ ] Aligned any UI changes with the [UI
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)

new-linked-folder is inside /zed-test/zed-project

output of ls -ld new-linked-folder 
`lrwxr-xr-x 1 prayanshchhablani staff 42 28 Mar 23:20 new-linked-folder
-> /Users/prayanshchhablani/new-target-folder`

this shows new-linked-folder is a symlink folder whose target is
new-target-folder which is outside the root dir of the project opened in
zed.



https://github.com/user-attachments/assets/ffebafc3-2fc4-4293-bdbf-3a894a45e276

Release Notes:

- Fixed file watching of symlinks that point outside of the
project/watched directory. Zed should now properly respond to changes in
files in symlinked directories

Change summary

crates/settings/src/settings_file.rs      |  36 +++
crates/worktree/src/worktree.rs           | 109 ++++++++++
crates/worktree/tests/integration/main.rs | 249 ++++++++++++++++++++++++
3 files changed, 392 insertions(+), 2 deletions(-)

Detailed changes

crates/settings/src/settings_file.rs 🔗

@@ -58,6 +58,41 @@ mod tests {
         fs.unpause_events_and_flush();
         assert_eq!(rx.next().await.as_deref(), Some("A"));
     }
+
+    #[gpui::test]
+    async fn test_watch_config_file_reloads_when_parent_dir_is_symlink(cx: &mut TestAppContext) {
+        cx.executor().allow_parking();
+        let fs = FakeFs::new(cx.background_executor.clone());
+        let config_settings_path = PathBuf::from("/root/.config/zed/settings.json");
+        let target_settings_path = PathBuf::from("/root/dotfiles/zed/settings.json");
+
+        fs.insert_tree(
+            Path::new("/root"),
+            json!({
+                ".config": {},
+                "dotfiles": {
+                    "zed": {
+                        "settings.json": "A"
+                    }
+                }
+            }),
+        )
+        .await;
+
+        fs.create_symlink(
+            Path::new("/root/.config/zed"),
+            PathBuf::from("/root/dotfiles/zed"),
+        )
+        .await
+        .unwrap();
+
+        let (mut rx, _task) =
+            watch_config_file(&cx.background_executor, fs.clone(), config_settings_path);
+        assert_eq!(rx.next().await.as_deref(), Some("A"));
+
+        fs.insert_file(&target_settings_path, b"B".to_vec()).await;
+        assert_eq!(rx.next().await.as_deref(), Some("B"));
+    }
 }
 
 pub const EMPTY_THEME_NAME: &str = "empty-theme";
@@ -134,6 +169,7 @@ pub fn watch_config_file(
 ) -> (mpsc::UnboundedReceiver<String>, gpui::Task<()>) {
     let (tx, rx) = mpsc::unbounded();
     let task = executor.spawn(async move {
+        let path = fs.canonicalize(&path).await.unwrap_or_else(|_| path);
         let (events, _) = fs.watch(&path, Duration::from_millis(100)).await;
         futures::pin_mut!(events);
 

crates/worktree/src/worktree.rs 🔗

@@ -260,7 +260,9 @@ pub struct LocalSnapshot {
 
 struct BackgroundScannerState {
     snapshot: LocalSnapshot,
+    symlink_paths_by_target: HashMap<Arc<Path>, SmallVec<[Arc<RelPath>; 1]>>,
     scanned_dirs: HashSet<ProjectEntryId>,
+    watched_dir_abs_paths_by_entry_id: HashMap<ProjectEntryId, Arc<Path>>,
     path_prefixes_to_scan: HashSet<Arc<RelPath>>,
     paths_to_scan: HashSet<Arc<RelPath>>,
     /// The ids of all of the entries that were removed from the snapshot
@@ -1171,7 +1173,9 @@ impl LocalWorktree {
                     state: async_lock::Mutex::new(BackgroundScannerState {
                         prev_snapshot: snapshot.snapshot.clone(),
                         snapshot,
+                        symlink_paths_by_target: Default::default(),
                         scanned_dirs: Default::default(),
+                        watched_dir_abs_paths_by_entry_id: Default::default(),
                         scanning_enabled,
                         path_prefixes_to_scan: Default::default(),
                         paths_to_scan: Default::default(),
@@ -3151,7 +3155,12 @@ impl BackgroundScannerState {
         let mut removed_dir_abs_paths = Vec::new();
         for entry in removed_entries.cursor::<()>(()) {
             if entry.is_dir() {
-                removed_dir_abs_paths.push(self.snapshot.absolutize(&entry.path));
+                let watch_path = self
+                    .watched_dir_abs_paths_by_entry_id
+                    .remove(&entry.id)
+                    .map(|path| path.as_ref().to_path_buf())
+                    .unwrap_or_else(|| self.snapshot.absolutize(&entry.path));
+                removed_dir_abs_paths.push(watch_path);
             }
 
             match self.removed_entries.entry(entry.inode) {
@@ -4194,6 +4203,67 @@ impl BackgroundScanner {
         self.send_status_update(scanning, request.done, &[]).await
     }
 
+    fn normalized_events_for_worktree(
+        state: &BackgroundScannerState,
+        root_canonical_path: &SanitizedPath,
+        mut events: Vec<PathEvent>,
+    ) -> Vec<PathEvent> {
+        if state.symlink_paths_by_target.is_empty() {
+            return events;
+        }
+        let mut mapped_events = Vec::new();
+
+        events.retain(|event| {
+            let abs_path = SanitizedPath::new(&event.path);
+
+            let mut best_match: Option<(&Arc<Path>, &SmallVec<[Arc<RelPath>; 1]>)> = None;
+            let mut best_depth = 0;
+            for (target_root, symlink_paths) in &state.symlink_paths_by_target {
+                if abs_path.as_path().starts_with(target_root.as_ref()) {
+                    let depth = target_root.as_ref().components().count();
+                    if depth > best_depth {
+                        best_depth = depth;
+                        best_match = Some((target_root, symlink_paths));
+                    }
+                }
+            }
+
+            let Some((target_root, symlink_paths)) = best_match else {
+                return true;
+            };
+
+            let Ok(suffix) = abs_path.as_path().strip_prefix(target_root.as_ref()) else {
+                return true;
+            };
+
+            // If the symlink's real target is outside this worktree, the original path
+            // isn't visible to the worktree. Keep only the remapped symlink events.
+            let keep_original = target_root.starts_with(root_canonical_path.as_path());
+
+            for symlink_path in symlink_paths {
+                let mapped_path = if suffix.as_os_str().is_empty() {
+                    root_canonical_path
+                        .as_path()
+                        .join(symlink_path.as_std_path())
+                } else {
+                    root_canonical_path
+                        .as_path()
+                        .join(symlink_path.as_std_path())
+                        .join(suffix)
+                };
+                if mapped_path != event.path {
+                    mapped_events.push(PathEvent {
+                        path: mapped_path,
+                        kind: event.kind,
+                    });
+                }
+            }
+            keep_original
+        });
+        events.extend(mapped_events);
+        events
+    }
+
     async fn process_events(&self, mut events: Vec<PathEvent>) {
         let root_path = self.state.lock().await.snapshot.abs_path.clone();
         let root_canonical_path = self.fs.canonicalize(root_path.as_path()).await;
@@ -4245,6 +4315,11 @@ impl BackgroundScanner {
             }
         };
 
+        {
+            let state = self.state.lock().await;
+            events = Self::normalized_events_for_worktree(&state, &root_canonical_path, events);
+        }
+
         // Certain directories may have FS changes, but do not lead to git data changes that Zed cares about.
         // Ignore these, to avoid Zed unnecessarily rescanning git metadata.
         let skipped_files_in_dot_git = [COMMIT_MESSAGE, INDEX_LOCK];
@@ -4513,7 +4588,15 @@ impl BackgroundScanner {
                     if let Some(entry) = state.snapshot.entry_for_path(ancestor)
                         && entry.kind == EntryKind::UnloadedDir
                     {
-                        let abs_path = root_path.join(ancestor.as_std_path());
+                        let abs_path = if entry.is_external {
+                            entry
+                                .canonical_path
+                                .as_ref()
+                                .map(|path| path.as_ref().to_path_buf())
+                                .unwrap_or_else(|| root_path.join(ancestor.as_std_path()))
+                        } else {
+                            root_path.join(ancestor.as_std_path())
+                        };
                         state
                             .enqueue_scan_dir(
                                 abs_path.into(),
@@ -4777,6 +4860,17 @@ impl BackgroundScanner {
                     child_entry.is_external = true;
                 }
 
+                if child_metadata.is_dir {
+                    let mut state = self.state.lock().await;
+                    let paths = state
+                        .symlink_paths_by_target
+                        .entry(Arc::from(canonical_path.clone()))
+                        .or_default();
+                    if !paths.iter().any(|path| path == &child_path) {
+                        paths.push(child_path.clone());
+                    }
+                }
+
                 child_entry.canonical_path = Some(canonical_path.into());
             }
 
@@ -4852,8 +4946,19 @@ impl BackgroundScanner {
         }
 
         state.populate_dir(job.path.clone(), new_entries, new_ignore);
+
         self.watcher.add(job.abs_path.as_ref()).log_err();
 
+        let entry_id = state
+            .snapshot
+            .entry_for_path(&job.path)
+            .map(|entry| entry.id);
+        if let Some(entry_id) = entry_id {
+            state
+                .watched_dir_abs_paths_by_entry_id
+                .insert(entry_id, job.abs_path.clone());
+        }
+
         for new_job in new_jobs.into_iter().flatten() {
             job.scan_queue
                 .try_send(new_job)

crates/worktree/tests/integration/main.rs 🔗

@@ -199,6 +199,9 @@ async fn test_symlinks_pointing_outside(cx: &mut TestAppContext) {
                 "src": {
                     "e.rs": "",
                     "f.rs": "",
+                    "nested": {
+                        "deep.rs": ""
+                    }
                 },
             }
         }),
@@ -212,6 +215,18 @@ async fn test_symlinks_pointing_outside(cx: &mut TestAppContext) {
     fs.create_symlink("/root/dir1/deps/dep-dir3".as_ref(), "../../dir3".into())
         .await
         .unwrap();
+    fs.create_symlink(
+        "/root/dir1/deps/dep-dir3-alias".as_ref(),
+        "../../dir3".into(),
+    )
+    .await
+    .unwrap();
+    fs.create_symlink(
+        "/root/dir1/deps/dep-dir3-nested".as_ref(),
+        "../../dir3/src/nested".into(),
+    )
+    .await
+    .unwrap();
 
     let tree = Worktree::local(
         Path::new("/root/dir1"),
@@ -254,6 +269,8 @@ async fn test_symlinks_pointing_outside(cx: &mut TestAppContext) {
                 (rel_path("deps"), false),
                 (rel_path("deps/dep-dir2"), true),
                 (rel_path("deps/dep-dir3"), true),
+                (rel_path("deps/dep-dir3-alias"), true),
+                (rel_path("deps/dep-dir3-nested"), true),
                 (rel_path("src"), false),
                 (rel_path("src/a.rs"), false),
                 (rel_path("src/b.rs"), false),
@@ -289,6 +306,8 @@ async fn test_symlinks_pointing_outside(cx: &mut TestAppContext) {
                 (rel_path("deps/dep-dir3"), true),
                 (rel_path("deps/dep-dir3/deps"), true),
                 (rel_path("deps/dep-dir3/src"), true),
+                (rel_path("deps/dep-dir3-alias"), true),
+                (rel_path("deps/dep-dir3-nested"), true),
                 (rel_path("src"), false),
                 (rel_path("src/a.rs"), false),
                 (rel_path("src/b.rs"), false),
@@ -328,6 +347,9 @@ async fn test_symlinks_pointing_outside(cx: &mut TestAppContext) {
                 (rel_path("deps/dep-dir3/src"), true),
                 (rel_path("deps/dep-dir3/src/e.rs"), true),
                 (rel_path("deps/dep-dir3/src/f.rs"), true),
+                (rel_path("deps/dep-dir3/src/nested"), true),
+                (rel_path("deps/dep-dir3-alias"), true),
+                (rel_path("deps/dep-dir3-nested"), true),
                 (rel_path("src"), false),
                 (rel_path("src/a.rs"), false),
                 (rel_path("src/b.rs"), false),
@@ -346,9 +368,220 @@ async fn test_symlinks_pointing_outside(cx: &mut TestAppContext) {
             (
                 rel_path("deps/dep-dir3/src/f.rs").into(),
                 PathChange::Loaded
+            ),
+            (
+                rel_path("deps/dep-dir3/src/nested").into(),
+                PathChange::Loaded
             )
         ]
     );
+
+    // After an external symlink subtree is loaded, changes in the target should be reflected.
+    fs.insert_file(Path::new("/root/dir3/src/new.rs"), b"".to_vec())
+        .await;
+
+    wait_for_condition(cx, |cx| {
+        tree.read_with(cx, |tree, _| {
+            tree.entry_for_path(rel_path("deps/dep-dir3/src/new.rs"))
+                .is_some()
+        })
+    })
+    .await;
+
+    tree.read_with(cx, |tree, _| {
+        assert!(
+            tree.entry_for_path(rel_path("deps/dep-dir3/src/new.rs"))
+                .is_some()
+        );
+    });
+
+    tree.read_with(cx, |tree, _| {
+        tree.as_local()
+            .unwrap()
+            .refresh_entries_for_paths(vec![rel_path("deps/dep-dir3-alias").into()])
+    })
+    .recv()
+    .await;
+
+    tree.read_with(cx, |tree, _| {
+        tree.as_local()
+            .unwrap()
+            .refresh_entries_for_paths(vec![rel_path("deps/dep-dir3-alias/src").into()])
+    })
+    .recv()
+    .await;
+
+    tree.read_with(cx, |tree, _| {
+        tree.as_local()
+            .unwrap()
+            .refresh_entries_for_paths(vec![rel_path("deps/dep-dir3-nested").into()])
+    })
+    .recv()
+    .await;
+    // Create a file in the shared target subtree. Because dep-dir3 and dep-dir3-alias both
+    // point to the same target, both logical paths should observe the new file.
+    fs.insert_file(Path::new("/root/dir3/src/shared-new.rs"), b"".to_vec())
+        .await;
+
+    wait_for_condition(cx, |cx| {
+        tree.read_with(cx, |tree, _| {
+            tree.entry_for_path(rel_path("deps/dep-dir3/src/shared-new.rs"))
+                .is_some()
+                && tree
+                    .entry_for_path(rel_path("deps/dep-dir3-alias/src/shared-new.rs"))
+                    .is_some()
+        })
+    })
+    .await;
+
+    tree.read_with(cx, |tree, _| {
+        assert!(
+            tree.entry_for_path(rel_path("deps/dep-dir3/src/shared-new.rs"))
+                .is_some()
+        );
+        assert!(
+            tree.entry_for_path(rel_path("deps/dep-dir3-alias/src/shared-new.rs"))
+                .is_some()
+        );
+    });
+
+    // Create a file under the more specific nested target. Longest-prefix matching means this should appear under dep-dir3-nested
+    fs.insert_file(
+        Path::new("/root/dir3/src/nested/longest-prefix.rs"),
+        b"".to_vec(),
+    )
+    .await;
+
+    wait_for_condition(cx, |cx| {
+        tree.read_with(cx, |tree, _| {
+            tree.entry_for_path(rel_path("deps/dep-dir3-nested/longest-prefix.rs"))
+                .is_some()
+        })
+    })
+    .await;
+
+    tree.read_with(cx, |tree, _| {
+        assert!(
+            tree.entry_for_path(rel_path("deps/dep-dir3-nested/longest-prefix.rs"))
+                .is_some()
+        );
+        assert!(
+            tree.entry_for_path(rel_path("deps/dep-dir3/src/nested/longest-prefix.rs"))
+                .is_none()
+        );
+        assert!(
+            tree.entry_for_path(rel_path("deps/dep-dir3-alias/src/nested/longest-prefix.rs"))
+                .is_none()
+        );
+    });
+}
+
+#[gpui::test]
+async fn test_symlinked_dir_inside_project(cx: &mut TestAppContext) {
+    init_test(cx);
+    let fs = FakeFs::new(cx.background_executor.clone());
+
+    fs.insert_tree(
+        "/root",
+        json!({
+            "project": {
+                "real-dir": {
+                    "existing.rs": "",
+                    "nested": {
+                        "deep.rs": ""
+                    }
+                },
+                "links": {}
+            }
+        }),
+    )
+    .await;
+
+    fs.create_symlink(
+        "/root/project/links/internal".as_ref(),
+        "../real-dir".into(),
+    )
+    .await
+    .unwrap();
+
+    let tree = Worktree::local(
+        Path::new("/root/project"),
+        true,
+        fs.clone(),
+        Default::default(),
+        true,
+        WorktreeId::from_proto(0),
+        &mut cx.to_async(),
+    )
+    .await
+    .unwrap();
+
+    cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete())
+        .await;
+
+    tree.read_with(cx, |tree, _| {
+        assert_eq!(
+            tree.entries(true, 0)
+                .map(|entry| (entry.path.as_ref(), entry.is_external))
+                .collect::<Vec<_>>(),
+            vec![
+                (rel_path(""), false),
+                (rel_path("links"), false),
+                (rel_path("links/internal"), false),
+                (rel_path("links/internal/existing.rs"), false),
+                (rel_path("links/internal/nested"), false),
+                (rel_path("links/internal/nested/deep.rs"), false),
+                (rel_path("real-dir"), false),
+                (rel_path("real-dir/existing.rs"), false),
+                (rel_path("real-dir/nested"), false),
+                (rel_path("real-dir/nested/deep.rs"), false),
+            ]
+        );
+
+        assert_eq!(
+            tree.entry_for_path(rel_path("links/internal"))
+                .unwrap()
+                .kind,
+            EntryKind::Dir
+        );
+    });
+
+    fs.insert_file(Path::new("/root/project/real-dir/new.txt"), b"".to_vec())
+        .await;
+    wait_for_condition(cx, |cx| {
+        tree.read_with(cx, |tree, _| {
+            tree.entry_for_path(rel_path("links/internal/new.txt"))
+                .is_some()
+        })
+    })
+    .await;
+
+    tree.read_with(cx, |tree, _| {
+        assert!(
+            tree.entry_for_path(rel_path("links/internal/new.txt"))
+                .is_some()
+        );
+    });
+
+    fs.insert_file(
+        Path::new("/root/project/real-dir/nested/inner.txt"),
+        b"".to_vec(),
+    )
+    .await;
+    wait_for_condition(cx, |cx| {
+        tree.read_with(cx, |tree, _| {
+            tree.entry_for_path(rel_path("links/internal/nested/inner.txt"))
+                .is_some()
+        })
+    })
+    .await;
+
+    tree.read_with(cx, |tree, _| {
+        assert!(
+            tree.entry_for_path(rel_path("links/internal/nested/inner.txt"))
+                .is_some()
+        );
+    });
 }
 
 #[cfg(target_os = "macos")]
@@ -2994,6 +3227,22 @@ fn init_test(cx: &mut gpui::TestAppContext) {
     });
 }
 
+async fn wait_for_condition(
+    cx: &mut TestAppContext,
+    mut condition: impl FnMut(&mut TestAppContext) -> bool,
+) {
+    for _ in 0..50 {
+        if condition(cx) {
+            return;
+        }
+        cx.executor().run_until_parked();
+        cx.background_executor
+            .timer(std::time::Duration::from_millis(10))
+            .await;
+    }
+    panic!("timed out waiting for test condition");
+}
+
 #[gpui::test]
 async fn test_load_file_encoding(cx: &mut TestAppContext) {
     init_test(cx);