From ce732be9913e72c03330e6740f1cc07bc8b55fd2 Mon Sep 17 00:00:00 2001 From: loadingalias <138315197+loadingalias@users.noreply.github.com> Date: Mon, 29 Jun 2026 05:49:01 -0400 Subject: [PATCH] terminal_view: Resolve remote terminal paths outside worktrees (#52537) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #39159 ## Summary • resolve remote terminal path candidates outside known worktrees through `Project::resolve_abs_path` • keep the local filesystem backed resolution path unchanged • preserve the existing preference for direct in-worktree matches when the terminal cwd is already inside a worktree • add regression coverage for both absolute and cwd-relative remote external paths ### Why The hyperlink parser wasn't the bug. `terminal_path_like_target` already knew how to parse these paths. The actual problem was that remote projects had no equivalent of the local background existence check. After worktree lookup failed, remote projects only fell back to worktree traversal, so cmd-click on `/tmp/a.txt` in an SSH remote terminal no-op'd. The project layer already has remote absolute-path resolution via `Project::resolve_abs_path`, so the right fix is to use that existing path for remote candidates instead of pretending remote projects cannot resolve paths outside their worktrees. ## Verification • `cargo test -p terminal_view terminal_path_like_target --quiet` • `cargo test -p terminal_view --quiet` ## Manual • Verified locally on my MBP w/ a stateless Zed build using SSH to self (`localhost`) • Opened `/tmp/zed-39159-project` as a remote project • Confirmed cmd-click opens `/tmp/zed-39159-external/a.txt` • Confirmed cmd-click opens `zed-39159-external/b.txt` from `cwd=/tmp` • Confirmed in-worktree paths still open correctly • Confirmed nonexistent external paths do not open bogus targets Release Notes: - Fixed remote terminal path opening so cmd-click can open files outside the project worktree. --- .../src/terminal_path_like_target.rs | 87 ++++-- crates/workspace/src/path_link.rs | 292 +++++++++++------- 2 files changed, 245 insertions(+), 134 deletions(-) diff --git a/crates/terminal_view/src/terminal_path_like_target.rs b/crates/terminal_view/src/terminal_path_like_target.rs index c90a2ce3d9..ac06721600 100644 --- a/crates/terminal_view/src/terminal_path_like_target.rs +++ b/crates/terminal_view/src/terminal_path_like_target.rs @@ -9,7 +9,7 @@ use util::{ResultExt, debug_panic}; use workspace::path_link::possible_open_target; #[cfg(test)] use workspace::path_link::{ - BackgroundFsChecks, OpenTargetFoundBy, possible_open_target_with_fs_checks, + BackgroundPathChecks, OpenTargetFoundBy, possible_open_target_with_fs_checks, }; use workspace::{OpenOptions, OpenVisible, Workspace, path_link::OpenTarget}; @@ -30,7 +30,7 @@ pub(super) fn hover_path_like_target( hovered_word, path_like_target, cx, - BackgroundFsChecks::Enabled, + BackgroundPathChecks::LocalFileSystem, ) } } @@ -40,7 +40,7 @@ fn possible_hover_target( hovered_word: HoveredWord, path_like_target: &PathLikeTarget, cx: &mut Context, - #[cfg(test)] background_fs_checks: BackgroundFsChecks, + #[cfg(test)] background_path_checks: BackgroundPathChecks, ) -> Task<()> { #[cfg(not(test))] let file_to_open_task = possible_open_target( @@ -55,13 +55,13 @@ fn possible_hover_target( &path_like_target.maybe_path, path_like_target.terminal_dir.as_deref(), cx, - background_fs_checks, + background_path_checks, ); cx.spawn(async move |terminal_view, cx| { let file_to_open = file_to_open_task.await; terminal_view .update(cx, |terminal_view, _| match file_to_open { - Some(OpenTarget::File(path, _) | OpenTarget::Worktree(path, ..)) => { + Some(OpenTarget::Path(path, ..) | OpenTarget::Worktree(path, ..)) => { terminal_view.hover = Some(HoverTarget { tooltip: path .to_string(&|path: &PathBuf| path.to_string_lossy().into_owned()), @@ -96,7 +96,7 @@ pub(super) fn open_path_like_target( path_like_target, window, cx, - BackgroundFsChecks::Enabled, + BackgroundPathChecks::LocalFileSystem, ) .detach_and_log_err(cx) } @@ -108,7 +108,7 @@ fn possibly_open_target( path_like_target: &PathLikeTarget, window: &mut Window, cx: &mut Context, - #[cfg(test)] background_fs_checks: BackgroundFsChecks, + #[cfg(test)] background_path_checks: BackgroundPathChecks, ) -> Task>> { if terminal_view.hover.is_none() { return Task::ready(Ok(None)); @@ -134,7 +134,7 @@ fn possibly_open_target( &path_like_target.maybe_path, path_like_target.terminal_dir.as_deref(), cx, - background_fs_checks, + background_path_checks, ) } })? @@ -224,7 +224,7 @@ mod tests { ) -> impl AsyncFnMut( HoveredWord, PathLikeTarget, - BackgroundFsChecks, + BackgroundPathChecks, ) -> (Option, Option) { let fs = app_cx.update(AppState::test).fs.as_fake().clone(); @@ -274,7 +274,7 @@ mod tests { async move |hovered_word: HoveredWord, path_like_target: PathLikeTarget, - background_fs_checks: BackgroundFsChecks| + background_path_checks: BackgroundPathChecks| -> (Option, Option) { let workspace_a = workspace.clone(); terminal_view @@ -284,7 +284,7 @@ mod tests { hovered_word, &path_like_target, cx, - background_fs_checks, + background_path_checks, ) }) .await; @@ -300,7 +300,7 @@ mod tests { &path_like_target, window, cx, - background_fs_checks, + background_path_checks, ) }) .await @@ -314,13 +314,13 @@ mod tests { test_path_like: &mut impl AsyncFnMut( HoveredWord, PathLikeTarget, - BackgroundFsChecks, + BackgroundPathChecks, ) -> (Option, Option), maybe_path: &str, tooltip: &str, terminal_dir: Option, - background_fs_checks: BackgroundFsChecks, - mut open_target_found_by: OpenTargetFoundBy, + background_path_checks: BackgroundPathChecks, + open_target_found_by: OpenTargetFoundBy, file: &str, line: u32, ) { @@ -334,7 +334,7 @@ mod tests { maybe_path: maybe_path.to_string(), terminal_dir, }, - background_fs_checks, + background_path_checks, ) .await; @@ -369,12 +369,6 @@ mod tests { "Open target path mismatch at {file}:{line}:" ); - if background_fs_checks == BackgroundFsChecks::Disabled - && open_target_found_by == OpenTargetFoundBy::FileSystemBackground - { - open_target_found_by = OpenTargetFoundBy::WorktreeScan; - } - assert_eq!( open_target.found_by(), open_target_found_by, @@ -404,7 +398,7 @@ mod tests { $maybe_path, $tooltip, $cwd, - BackgroundFsChecks::Enabled, + BackgroundPathChecks::LocalFileSystem, $found_by ); test_path_like!( @@ -412,7 +406,7 @@ mod tests { $maybe_path, $tooltip, $cwd, - BackgroundFsChecks::Disabled, + BackgroundPathChecks::ProjectPathResolution, $found_by ); }}; @@ -479,7 +473,7 @@ mod tests { $maybe_path, $tooltip, $cwd, - BackgroundFsChecks::Enabled, + BackgroundPathChecks::LocalFileSystem, OpenTargetFoundBy::WorktreeExact ) }; @@ -489,7 +483,7 @@ mod tests { $maybe_path, $tooltip, $cwd, - BackgroundFsChecks::Enabled, + BackgroundPathChecks::LocalFileSystem, OpenTargetFoundBy::$found_by ) } @@ -504,7 +498,7 @@ mod tests { $maybe_path, $tooltip, $cwd, - BackgroundFsChecks::Disabled, + BackgroundPathChecks::ProjectPathResolution, OpenTargetFoundBy::WorktreeExact ) }; @@ -514,7 +508,7 @@ mod tests { $maybe_path, $tooltip, $cwd, - BackgroundFsChecks::Disabled, + BackgroundPathChecks::ProjectPathResolution, OpenTargetFoundBy::$found_by ) } @@ -939,7 +933,7 @@ mod tests { vec![path!("/test")], { // Note: Opening a non-worktree file adds that file as a single file worktree. - test_local!("file.txt", "/file.txt", "/", FileSystemBackground); + test_local!("file.txt", "/file.txt", "/", BackgroundPathResolution); } ) } @@ -966,12 +960,45 @@ mod tests { vec![path!("/test")], { // Note: Opening a non-worktree file adds that file as a single file worktree. - test_remote!("file.txt", "/test/file.txt", "/"); + test_remote!("file.txt", "/file.txt", "/", BackgroundPathResolution); test_remote!("/test/file.txt", "/test/file.txt", "/"); } ) } + // https://github.com/zed-industries/zed/issues/39159 + #[gpui::test] + async fn issue_39159_remote_absolute_path_outside_worktree(cx: &mut TestAppContext) { + test_path_likes!( + cx, + vec![ + ( + path!("/tmp"), + json!({ + "a.txt": "", + }), + ), + ( + path!("/code/project"), + json!({ + "src": { + "lib.rs": "", + }, + }), + ), + ], + vec![path!("/code/project")], + { + test_remote!( + "/tmp/a.txt", + "/tmp/a.txt", + "/code/project", + BackgroundPathResolution + ); + } + ) + } + // See https://github.com/zed-industries/zed/issues/34027 #[gpui::test] #[should_panic(expected = "Tooltip mismatch")] diff --git a/crates/workspace/src/path_link.rs b/crates/workspace/src/path_link.rs index e707a7de60..6cbcfccb14 100644 --- a/crates/workspace/src/path_link.rs +++ b/crates/workspace/src/path_link.rs @@ -1,7 +1,7 @@ use crate::Workspace; -use gpui::{App, AppContext, Task, WeakEntity}; +use gpui::{App, AppContext, Entity, Task, WeakEntity}; use itertools::Itertools; -use project::{Entry, Metadata}; +use project::{Entry, Worktree}; use std::path::{Path, PathBuf}; use util::{ paths::{PathStyle, PathWithPosition, normalize_lexically}, @@ -13,13 +13,13 @@ use util::{ pub enum OpenTargetFoundBy { WorktreeExact, WorktreeScan, - FileSystemBackground, + BackgroundPathResolution, } #[derive(Debug, Clone, Copy, Eq, PartialEq)] -pub enum BackgroundFsChecks { - Enabled, - Disabled, +pub enum BackgroundPathChecks { + LocalFileSystem, + ProjectPathResolution, } #[derive(Debug, Clone)] @@ -29,28 +29,32 @@ pub enum OpenTarget { Entry, #[cfg(any(test, feature = "test-support"))] OpenTargetFoundBy, ), - File(PathWithPosition, Metadata), + Path( + PathWithPosition, + bool, + #[cfg(any(test, feature = "test-support"))] OpenTargetFoundBy, + ), } impl OpenTarget { pub fn is_file(&self) -> bool { match self { OpenTarget::Worktree(_, entry, ..) => entry.is_file(), - OpenTarget::File(_, metadata) => !metadata.is_dir, + OpenTarget::Path(_, is_dir, ..) => !is_dir, } } pub fn is_dir(&self) -> bool { match self { OpenTarget::Worktree(_, entry, ..) => entry.is_dir(), - OpenTarget::File(_, metadata) => metadata.is_dir, + OpenTarget::Path(_, is_dir, ..) => *is_dir, } } pub fn path(&self) -> &PathWithPosition { match self { OpenTarget::Worktree(path, ..) => path, - OpenTarget::File(path, _) => path, + OpenTarget::Path(path, ..) => path, } } @@ -58,7 +62,7 @@ impl OpenTarget { pub fn found_by(&self) -> OpenTargetFoundBy { match self { OpenTarget::Worktree(.., found_by) => *found_by, - OpenTarget::File(..) => OpenTargetFoundBy::FileSystemBackground, + OpenTarget::Path(.., found_by) => *found_by, } } } @@ -137,9 +141,9 @@ pub fn possible_open_target_with_fs_checks( maybe_path: &str, cwd: Option<&Path>, cx: &App, - background_fs_checks: BackgroundFsChecks, + background_path_checks: BackgroundPathChecks, ) -> Task> { - possible_open_target_internal(workspace, maybe_path, cwd, cx, Some(background_fs_checks)) + possible_open_target_internal(workspace, maybe_path, cwd, cx, Some(background_path_checks)) } fn possible_open_target_internal( @@ -147,7 +151,7 @@ fn possible_open_target_internal( maybe_path: &str, cwd: Option<&Path>, cx: &App, - background_fs_checks: Option, + background_path_checks: Option, ) -> Task> { let Some(workspace) = workspace.upgrade() else { return Task::ready(None); @@ -287,105 +291,96 @@ fn possible_open_target_internal( } } - let enable_background_fs_checks = background_fs_checks - .map(|background_fs_checks| background_fs_checks == BackgroundFsChecks::Enabled) - .unwrap_or_else(|| workspace.read(cx).project().read(cx).is_local()); - if open_target.is_some() { - if !enable_background_fs_checks || is_cwd_in_worktree { + if is_cwd_in_worktree { return Task::ready(open_target); } } - let fs_paths_to_check = if enable_background_fs_checks { - let fs_cwd_paths_to_check = cwd - .iter() - .flat_map(|cwd| { - let mut paths_to_check = Vec::new(); - for path_to_check in &potential_paths { - let maybe_path = &path_to_check.path; - if path_to_check.path.is_relative() { - paths_to_check.push(PathWithPosition { - path: cwd.join(maybe_path), - row: path_to_check.row, - column: path_to_check.column, - }); - } - } - paths_to_check - }) - .collect::>(); - fs_cwd_paths_to_check - .into_iter() - .chain( - potential_paths - .into_iter() - .flat_map(|path_to_check| { - let mut paths_to_check = Vec::new(); - let maybe_path = &path_to_check.path; - if maybe_path.starts_with("~") { - if let Some(home_path) = maybe_path - .strip_prefix("~") - .ok() - .and_then(|stripped| Some(dirs::home_dir()?.join(stripped))) - { - paths_to_check.push(PathWithPosition { - path: home_path, - row: path_to_check.row, - column: path_to_check.column, - }); - } - } else { - paths_to_check.push(PathWithPosition { - path: maybe_path.clone(), - row: path_to_check.row, - column: path_to_check.column, - }); - if maybe_path.is_relative() { - for worktree in &worktree_candidates { - if !worktree.read(cx).is_single_file() { - paths_to_check.push(PathWithPosition { - path: worktree.read(cx).abs_path().join(maybe_path), - row: path_to_check.row, - column: path_to_check.column, - }); - } - } - } - } - paths_to_check - }) - .collect::>(), - ) - .collect() - } else { - Vec::new() - }; - - let fs = workspace.read(cx).project().read(cx).fs().clone(); - let background_fs_checks_task = cx.background_spawn(async move { - for mut path_to_check in fs_paths_to_check { - if let Some(fs_path_to_check) = fs.canonicalize(&path_to_check.path).await.ok() - && let Some(metadata) = fs.metadata(&fs_path_to_check).await.ok().flatten() - { - if open_target - .as_ref() - .map(|open_target| open_target.path().path != fs_path_to_check) - .unwrap_or(true) - { - path_to_check.path = fs_path_to_check; - return Some(OpenTarget::File(path_to_check, metadata)); - } - - break; - } + let project = workspace.read(cx).project().clone(); + let background_path_checks = background_path_checks.unwrap_or_else(|| { + if project.read(cx).is_local() { + BackgroundPathChecks::LocalFileSystem + } else { + BackgroundPathChecks::ProjectPathResolution } - - open_target }); + let background_resolution_task = match background_path_checks { + BackgroundPathChecks::LocalFileSystem => { + let fs_paths_to_check = + local_paths_to_check(&potential_paths, cwd, &worktree_candidates, cx); + let fs = project.read(cx).fs().clone(); + cx.background_spawn(async move { + for mut path_to_check in fs_paths_to_check { + if let Some(fs_path_to_check) = fs.canonicalize(&path_to_check.path).await.ok() + && let Some(metadata) = fs.metadata(&fs_path_to_check).await.ok().flatten() + { + if open_target + .as_ref() + .map(|open_target| open_target.path().path != fs_path_to_check) + .unwrap_or(true) + { + path_to_check.path = fs_path_to_check; + return Some(OpenTarget::Path( + path_to_check, + metadata.is_dir, + #[cfg(any(test, feature = "test-support"))] + OpenTargetFoundBy::BackgroundPathResolution, + )); + } + + break; + } + } + + open_target + }) + } + BackgroundPathChecks::ProjectPathResolution => { + let paths_to_check = project_paths_to_check(&potential_paths, cwd); + cx.spawn(async move |cx| { + for mut path_to_check in paths_to_check { + let path = path_to_check.path.to_string_lossy(); + let resolve_task = project.update(cx, |project, cx| { + project.resolve_abs_path(path.as_ref(), cx) + }); + + if let Some(resolved_path) = resolve_task.await + && let Some(resolved_abs_path) = { + let is_dir = resolved_path.is_dir(); + resolved_path + .into_abs_path() + .map(|resolved_abs_path| (resolved_abs_path, is_dir)) + } + { + let (resolved_abs_path, is_dir) = resolved_abs_path; + let resolved_abs_path = PathBuf::from(resolved_abs_path); + if open_target + .as_ref() + .map(|open_target| open_target.path().path != resolved_abs_path) + .unwrap_or(true) + { + path_to_check.path = resolved_abs_path; + return Some(OpenTarget::Path( + path_to_check, + is_dir, + #[cfg(any(test, feature = "test-support"))] + OpenTargetFoundBy::BackgroundPathResolution, + )); + } + + break; + } + } + + open_target + }) + } + }; + cx.spawn(async move |cx| { - background_fs_checks_task.await.or_else(|| { + background_resolution_task.await.or_else(|| { for (worktree, worktree_paths_to_check) in worktree_paths_to_check { if let Some(found_entry) = worktree.update(cx, |worktree, _| -> Option { @@ -420,3 +415,92 @@ fn possible_open_target_internal( }) }) } + +fn local_paths_to_check( + potential_paths: &[PathWithPosition], + cwd: Option<&Path>, + worktree_candidates: &[Entity], + cx: &App, +) -> Vec { + cwd.iter() + .flat_map(|cwd| { + potential_paths.iter().filter_map(|path_to_check| { + path_to_check.path.is_relative().then(|| PathWithPosition { + path: cwd.join(&path_to_check.path), + row: path_to_check.row, + column: path_to_check.column, + }) + }) + }) + .chain(potential_paths.iter().flat_map(|path_to_check| { + let mut paths_to_check = Vec::new(); + let maybe_path = &path_to_check.path; + if maybe_path.starts_with("~") { + if let Some(home_path) = + maybe_path + .strip_prefix("~") + .ok() + .and_then(|stripped_maybe_path| { + Some(dirs::home_dir()?.join(stripped_maybe_path)) + }) + { + paths_to_check.push(PathWithPosition { + path: home_path, + row: path_to_check.row, + column: path_to_check.column, + }); + } + } else { + paths_to_check.push(PathWithPosition { + path: maybe_path.clone(), + row: path_to_check.row, + column: path_to_check.column, + }); + if maybe_path.is_relative() { + for worktree in worktree_candidates { + if !worktree.read(cx).is_single_file() { + paths_to_check.push(PathWithPosition { + path: worktree.read(cx).abs_path().join(maybe_path), + row: path_to_check.row, + column: path_to_check.column, + }); + } + } + } + } + paths_to_check + })) + .collect() +} + +fn project_paths_to_check( + potential_paths: &[PathWithPosition], + cwd: Option<&Path>, +) -> Vec { + cwd.iter() + .flat_map(|cwd| { + potential_paths + .iter() + .filter_map(|path_to_check| normalize_absolute_candidate(cwd, path_to_check)) + }) + .chain(potential_paths.iter().filter_map(|path_to_check| { + let maybe_path = &path_to_check.path; + (maybe_path.starts_with("~") || maybe_path.is_absolute()).then(|| path_to_check.clone()) + })) + .collect() +} + +fn normalize_absolute_candidate( + cwd: &Path, + path_to_check: &PathWithPosition, +) -> Option { + path_to_check.path.is_relative().then(|| { + normalize_lexically(&cwd.join(&path_to_check.path)) + .ok() + .map(|path| PathWithPosition { + path, + row: path_to_check.row, + column: path_to_check.column, + }) + })? +}