Fix agent file link navigation (#64748)

Closes https://github.com/zed-industries/zed/issues/56633
Related to https://github.com/zed-industries/zed/issues/56067
Related to https://github.com/zed-industries/zed/issues/63968

Changes path handling to open files in the agent panel similar to what
cmd-click does: regular open for files from the project, singe-file
worktrees for files outside of the project

Release Notes:

- Fixed agent file link navigation

crates/agent_ui/src/agent_ui.rs

@@ -43,8 +43,9 @@ use std::sync::Arc;
 use ::ui::IconName;
 use agent_client_protocol::schema::{v1 as acp_v1, v2 as acp_v2};
 use agent_settings::{AgentProfileId, AgentSettings};
+use anyhow::Context as _;
 use command_palette_hooks::CommandPaletteFilter;
-use editor::{Editor, SelectionEffects, scroll::Autoscroll};
+use editor::{Editor, scroll::Autoscroll};
 use feature_flags::FeatureFlagAppExt as _;
 use fs::Fs;
 use gpui::{
@@ -58,7 +59,7 @@ use language::{
 use language_model::{
     LanguageModel, LanguageModelId, LanguageModelProviderId, LanguageModelRegistry,
 };
-use project::{AgentId, DisableAiSettings};
+use project::{AgentId, DisableAiSettings, Project, ProjectPath};
 use prompt_store::{self, PromptBuilder, rules_to_skills_migration};
 use rope::Point;
 use schemars::JsonSchema;
@@ -66,7 +67,11 @@ use serde::{Deserialize, Serialize};
 use settings::{LanguageModelSelection, Settings as _, SettingsStore, SidebarSide};
 use std::any::TypeId;
 use std::path::{Path, PathBuf};
-use workspace::{OpenOptions, Workspace};
+use util::{
+    paths::SanitizedPath,
+    rel_path::{RelPath, RelPathBuf},
+};
+use workspace::Workspace;
 
 use crate::agent_configuration::ManageProfilesModal;
 pub use crate::agent_connection_store::{ActiveAcpConnection, AgentConnectionStore};
@@ -118,6 +123,72 @@ pub(crate) fn resolve_agent_image(
     None
 }
 
+pub(crate) fn project_path_for_file_link(
+    project: &Project,
+    path: &Path,
+    cx: &App,
+) -> Option<ProjectPath> {
+    let path_style = project.path_style(cx);
+    let validate_path = |project_path: ProjectPath, relative_path: &str| {
+        if project.get_open_buffer(&project_path, cx).is_none()
+            && !project.entry_for_path(&project_path, cx)?.is_file()
+        {
+            return None;
+        }
+        let worktree = project.worktree_for_id(project_path.worktree_id, cx)?;
+        let worktree = worktree.read(cx);
+        let mut prefix = RelPathBuf::new();
+        for component in relative_path.split(path_style.separators_ch()) {
+            match component {
+                "" | "." => {}
+                ".." => {
+                    let entry = worktree.entry_for_path(&prefix)?;
+                    if !entry.is_dir() || entry.canonical_path.is_some() || !prefix.pop() {
+                        return None;
+                    }
+                }
+                component => prefix.push_component(component).ok()?,
+            }
+        }
+        Some(project_path)
+    };
+    let path = SanitizedPath::new(path).as_path();
+    if path_style.is_absolute(path.to_str()?) {
+        let project_path = project.project_path_for_absolute_path(path, cx)?;
+        let worktree = project.worktree_for_id(project_path.worktree_id, cx)?;
+        let root = worktree.read(cx).abs_path();
+        let root_length = root
+            .to_str()?
+            .trim_end_matches(path_style.separators_ch())
+            .len();
+        let relative_path = path
+            .to_str()?
+            .get(root_length..)?
+            .trim_start_matches(path_style.separators_ch());
+        validate_path(project_path, relative_path)
+    } else {
+        project.visible_worktrees(cx).find_map(|worktree| {
+            let worktree = worktree.read(cx);
+            [
+                path.strip_prefix(worktree.root_name().as_std_path()).ok(),
+                Some(path),
+            ]
+            .into_iter()
+            .flatten()
+            .find_map(|relative_path| {
+                let path = RelPath::new(relative_path, path_style).ok()?.into_arc();
+                validate_path(
+                    ProjectPath {
+                        worktree_id: worktree.id(),
+                        path,
+                    },
+                    relative_path.to_str()?,
+                )
+            })
+        })
+    }
+}
+
 /// Opens `abs_path` in the workspace, moving the cursor to `point` when one
 /// is given. Paths outside every worktree are only opened when a file exists
 /// there, so broken agent links don't create empty buffers.
@@ -128,39 +199,47 @@ pub(crate) fn open_abs_path_at_point(
     window: &mut Window,
     cx: &mut Context<Workspace>,
 ) {
-    let project_path = workspace
-        .project()
-        .update(cx, |project, cx| project.find_project_path(&abs_path, cx));
-    let fs = workspace.project().read(cx).fs().clone();
+    let project_path = workspace.project().read_with(cx, |project, cx| {
+        project_path_for_file_link(project, &abs_path, cx)
+    });
+    let project = workspace.project().downgrade();
     let workspace = cx.weak_entity();
     window
         .spawn(cx, async move |cx| {
-            let item = if let Some(project_path) = project_path {
+            let task = if let Some(project_path) = project_path {
                 workspace
                     .update_in(cx, |workspace, window, cx| {
                         workspace.open_path(project_path, None, true, window, cx)
-                    })?
-                    .await?
+                    })
+                    .ok()
             } else {
-                let metadata = fs.metadata(&abs_path).await?;
-                anyhow::ensure!(
-                    metadata.is_some_and(|metadata| !metadata.is_dir),
-                    "no file found at path {abs_path:?}"
-                );
+                let path = abs_path
+                    .to_str()
+                    .context("file link path is not valid UTF-8")?;
+                let Ok(task) =
+                    project.update(cx, |project, cx| project.resolve_abs_file_link(path, cx))
+                else {
+                    return Ok(());
+                };
+                let Some(resolved_path) = task
+                    .await
+                    .with_context(|| format!("resolving agent file link {abs_path:?}"))?
+                else {
+                    log::warn!(
+                        "Could not resolve agent file link to {abs_path:?}: no matching file"
+                    );
+                    return Ok(());
+                };
                 workspace
                     .update_in(cx, |workspace, window, cx| {
-                        workspace.open_abs_path(
-                            abs_path,
-                            OpenOptions {
-                                focus: Some(true),
-                                ..Default::default()
-                            },
-                            window,
-                            cx,
-                        )
-                    })?
-                    .await?
+                        workspace.open_resolved_path(resolved_path, window, cx)
+                    })
+                    .ok()
             };
+            let Some(task) = task else {
+                return Ok(());
+            };
+            let item = task.await?;
             let Some(point) = point else {
                 return Ok(());
             };
@@ -169,12 +248,14 @@ pub(crate) fn open_abs_path_at_point(
             };
             editor
                 .update_in(cx, |editor, window, cx| {
-                    editor.change_selections(
-                        SelectionEffects::scroll(Autoscroll::center()),
-                        window,
-                        cx,
-                        |selections| selections.select_ranges([point..point]),
-                    );
+                    if let Some(buffer) = editor.buffer().read(cx).as_singleton() {
+                        let point = buffer
+                            .read(cx)
+                            .snapshot()
+                            .point_from_external_input(point.row, point.column);
+                        editor.go_to_singleton_buffer_point(point, window, cx);
+                        editor.request_autoscroll(Autoscroll::center(), cx);
+                    }
                 })
                 .ok();
             anyhow::Ok(())

crates/project/src/project.rs

@@ -4978,6 +4978,122 @@ impl Project {
         })
     }
 
+    pub fn resolve_abs_file_link(
+        &self,
+        path: &str,
+        cx: &mut Context<Self>,
+    ) -> Task<Result<Option<ResolvedPath>>> {
+        let resolve_task = self.resolve_abs_file_path_canonical(path, cx);
+        let path = if self.is_local() {
+            shellexpand::tilde(path).into_owned()
+        } else {
+            path.to_owned()
+        };
+        cx.spawn(async move |project, cx| {
+            let Some(resolved_path) = resolve_task.await? else {
+                return Ok(None);
+            };
+            let path = if path.starts_with("~") {
+                let Ok(task) =
+                    project.update(cx, |project, cx| project.resolve_abs_path(&path, cx))
+                else {
+                    return Ok(None);
+                };
+                let Some(path) = task.await.and_then(ResolvedPath::into_abs_path) else {
+                    return Ok(Some(resolved_path));
+                };
+                path
+            } else {
+                path
+            };
+            let Ok(candidate) = project.update(cx, |project, cx| {
+                let lexical_path = project.path_style(cx).normalize(&path);
+                let project_path =
+                    project.project_path_for_absolute_path(Path::new(&lexical_path), cx)?;
+                let abs_path = project.absolute_path(&project_path, cx)?;
+                let task = project.resolve_abs_file_path_canonical(abs_path.to_str()?, cx);
+                Some((project_path, abs_path, task))
+            }) else {
+                return Ok(None);
+            };
+            let Some((project_path, abs_path, task)) = candidate else {
+                return Ok(Some(resolved_path));
+            };
+            let canonical_candidate = task
+                .await
+                .with_context(|| format!("validating file link alias {abs_path:?}"))
+                .log_err()
+                .flatten();
+            let Ok(alias_unchanged) = project.read_with(cx, |project, cx| {
+                project.absolute_path(&project_path, cx).as_deref() == Some(abs_path.as_path())
+            }) else {
+                return Ok(None);
+            };
+            if alias_unchanged
+                && canonical_candidate
+                    .is_some_and(|candidate| candidate.abs_path() == resolved_path.abs_path())
+            {
+                Ok(Some(ResolvedPath::ProjectPath {
+                    project_path,
+                    is_dir: false,
+                }))
+            } else {
+                Ok(Some(resolved_path))
+            }
+        })
+    }
+
+    pub fn resolve_abs_file_path_canonical(
+        &self,
+        path: &str,
+        cx: &mut Context<Self>,
+    ) -> Task<Result<Option<ResolvedPath>>> {
+        if self.is_local() {
+            let expanded = PathBuf::from(shellexpand::tilde(path).into_owned());
+            let fs = self.fs.clone();
+            cx.background_spawn(async move {
+                if fs
+                    .metadata(&expanded)
+                    .await?
+                    .is_none_or(|metadata| metadata.is_dir)
+                {
+                    return Ok(None);
+                }
+                let path = fs.canonicalize(&expanded).await?;
+                let path = path
+                    .to_str()
+                    .context("canonical file path is not valid UTF-8")?
+                    .to_owned();
+                Ok(Some(ResolvedPath::AbsPath {
+                    path,
+                    is_dir: false,
+                }))
+            })
+        } else if let Some(ssh_client) = self.remote_client.as_ref() {
+            let request = ssh_client
+                .read(cx)
+                .proto_client()
+                .request(proto::GetPathMetadata {
+                    project_id: REMOTE_SERVER_PROJECT_ID,
+                    path: path.into(),
+                    canonicalize: true,
+                });
+            cx.background_spawn(async move {
+                let response = request.await?;
+                if response.exists && !response.is_dir {
+                    Ok(Some(ResolvedPath::AbsPath {
+                        path: response.path,
+                        is_dir: false,
+                    }))
+                } else {
+                    Ok(None)
+                }
+            })
+        } else {
+            Task::ready(Ok(None))
+        }
+    }
+
     pub fn resolve_abs_path(&self, path: &str, cx: &App) -> Task<Option<ResolvedPath>> {
         if self.is_local() {
             let expanded = PathBuf::from(shellexpand::tilde(&path).into_owned());
@@ -4997,6 +5113,7 @@ impl Project {
                 .request(proto::GetPathMetadata {
                     project_id: REMOTE_SERVER_PROJECT_ID,
                     path: path.into(),
+                    canonicalize: false,
                 });
             cx.background_spawn(async move {
                 let response = request.await.log_err()?;

crates/project/tests/integration/project_tests.rs

@@ -85,6 +85,8 @@ use std::{
     task::Poll,
     time::Duration,
 };
+#[cfg(target_os = "linux")]
+use std::{ffi::OsStr, os::unix::ffi::OsStrExt as _};
 use sum_tree::SumTree;
 use task::{ResolvedTask, ShellKind, TaskContext};
 use text::{Anchor, PointUtf16, ReplicaId, ToOffset, Unclipped};
@@ -239,6 +241,153 @@ async fn test_symlinks(cx: &mut gpui::TestAppContext) {
     });
 }
 
+#[cfg(not(windows))]
+#[gpui::test]
+async fn test_resolve_abs_file_link_project_aliases(cx: &mut gpui::TestAppContext) {
+    init_test(cx);
+    cx.executor().allow_parking();
+    cx.update(|cx| {
+        SettingsStore::update_global(cx, |store, cx| {
+            store.update_user_settings(cx, |settings| {
+                settings.project.worktree.file_scan_exclusions =
+                    Some(SplicingVec::from(vec!["**/hidden".to_string()]));
+            });
+        });
+    });
+
+    let directory = TempTree::new(json!({
+        "project": {
+            "sub": {},
+            "hidden": {"unopened.rs": "unindexed"},
+        },
+        "outside": {
+            "child": {},
+            "target.rs": "saved",
+            "link.rs": "actual outside target",
+        },
+    }));
+    let root = directory.path().join("project");
+    let target = directory.path().join("outside/target.rs");
+    os::unix::fs::symlink(&target, root.join("link.rs")).unwrap();
+    os::unix::fs::symlink(root.join("sub"), root.join("hop")).unwrap();
+    os::unix::fs::symlink(directory.path().join("outside/child"), root.join("escape")).unwrap();
+    let outside_alias = directory.path().join("outside/alias.rs");
+    os::unix::fs::symlink(&target, &outside_alias).unwrap();
+
+    let project = Project::test(RealFs::new(None, cx.executor()), [root.as_path()], cx).await;
+    let alias = project.read_with(cx, |project, cx| {
+        project
+            .project_path_for_absolute_path(&root.join("link.rs"), cx)
+            .unwrap()
+    });
+    let buffer = project
+        .update(cx, |project, cx| project.open_buffer(alias.clone(), cx))
+        .await
+        .unwrap();
+    buffer.update(cx, |buffer, cx| {
+        buffer.edit([(0..0, "unsaved ")], None, cx);
+    });
+
+    for relative_path in ["hop/../link.rs", "hidden/../link.rs"] {
+        let path = root.join(relative_path);
+        let resolved = project
+            .update(cx, |project, cx| {
+                project.resolve_abs_file_link(path.to_str().unwrap(), cx)
+            })
+            .await
+            .unwrap()
+            .unwrap();
+        assert_eq!(resolved.project_path(), Some(&alias), "{relative_path}");
+        let reopened = project
+            .update(cx, |project, cx| {
+                project.open_buffer(resolved.project_path().unwrap().clone(), cx)
+            })
+            .await
+            .unwrap();
+        assert_eq!(reopened, buffer);
+        buffer.read_with(cx, |buffer, _| {
+            assert_eq!(buffer.text(), "unsaved saved");
+            assert!(buffer.is_dirty());
+        });
+    }
+
+    let unindexed_path = root.join("hidden/unopened.rs");
+    let unindexed_alias = project.read_with(cx, |project, cx| {
+        let path = project
+            .project_path_for_absolute_path(&unindexed_path, cx)
+            .unwrap();
+        assert!(project.entry_for_path(&path, cx).is_none());
+        assert!(project.get_open_buffer(&path, cx).is_none());
+        assert!(
+            project
+                .entry_for_path(&(alias.worktree_id, rel_path("hidden")).into(), cx)
+                .is_none()
+        );
+        path
+    });
+    let resolved = project
+        .update(cx, |project, cx| {
+            project.resolve_abs_file_link(unindexed_path.to_str().unwrap(), cx)
+        })
+        .await
+        .unwrap()
+        .unwrap();
+    assert_eq!(resolved.project_path(), Some(&unindexed_alias));
+
+    for (input, expected) in [
+        (
+            root.join("escape/../link.rs"),
+            directory.path().join("outside/link.rs"),
+        ),
+        (outside_alias, target),
+    ] {
+        let resolved = project
+            .update(cx, |project, cx| {
+                project.resolve_abs_file_link(input.to_str().unwrap(), cx)
+            })
+            .await
+            .unwrap()
+            .unwrap();
+        assert_eq!(resolved.abs_path(), expected.to_str());
+    }
+    let missing = root.join("missing/../link.rs");
+    let resolved = project
+        .update(cx, |project, cx| {
+            project.resolve_abs_file_link(missing.to_str().unwrap(), cx)
+        })
+        .await
+        .unwrap();
+    assert!(resolved.is_none());
+    project.read_with(cx, |project, cx| {
+        assert_eq!(project.worktrees(cx).count(), 1);
+        assert_eq!(project.get_open_buffer(&alias, cx), Some(buffer));
+    });
+}
+
+#[cfg(target_os = "linux")]
+#[gpui::test]
+async fn test_resolve_abs_file_link_rejects_non_utf8_target(cx: &mut gpui::TestAppContext) {
+    init_test(cx);
+    cx.executor().allow_parking();
+    let directory = TempTree::new(json!({}));
+    let target = directory.path().join(OsStr::from_bytes(b"\xff.rs"));
+    std::fs::write(&target, "actual target").unwrap();
+    std::fs::write(directory.path().join("<EFBFBD>.rs"), "replacement-character decoy").unwrap();
+    let alias = directory.path().join("alias.rs");
+    os::unix::fs::symlink(&target, &alias).unwrap();
+    let project = Project::test(RealFs::new(None, cx.executor()), [], cx).await;
+    let error = project
+        .update(cx, |project, cx| {
+            project.resolve_abs_file_link(alias.to_str().unwrap(), cx)
+        })
+        .await
+        .unwrap_err();
+    assert_eq!(error.to_string(), "canonical file path is not valid UTF-8");
+    project.read_with(cx, |project, cx| {
+        assert_eq!(project.worktrees(cx).count(), 0);
+    });
+}
+
 #[gpui::test]
 async fn test_editorconfig_support(cx: &mut gpui::TestAppContext) {
     init_test(cx);

crates/proto/proto/worktree.proto

@@ -52,6 +52,7 @@ message RemoveWorktree {
 message GetPathMetadata {
   uint64 project_id = 1;
   string path = 2;
+  bool canonicalize = 3;
 }
 
 message GetPathMetadataResponse {

crates/remote_server/src/headless_project.rs

@@ -1265,11 +1265,20 @@ impl HeadlessProject {
 
         let metadata = fs.metadata(&expanded).await?;
         let is_dir = metadata.map(|metadata| metadata.is_dir).unwrap_or(false);
+        let path = if envelope.payload.canonicalize && metadata.is_some() {
+            fs.canonicalize(&expanded)
+                .await?
+                .to_str()
+                .context("canonical file path is not valid UTF-8")?
+                .to_owned()
+        } else {
+            expanded.to_string_lossy().into_owned()
+        };
 
         Ok(proto::GetPathMetadataResponse {
             exists: metadata.is_some(),
             is_dir,
-            path: expanded.to_string_lossy().into_owned(),
+            path,
         })
     }
 

crates/remote_server/src/remote_editing_tests.rs

@@ -2871,9 +2871,17 @@ async fn test_remote_resolve_abs_path(cx: &mut TestAppContext, server_cx: &mut T
                     "lib.rs": "fn one() -> usize { 1 }"
                 }
             },
+            "project2": {
+                "README.md": "# project 2",
+            },
         }),
     )
     .await;
+    fs.insert_symlink(
+        path!("/code/project1/linked"),
+        PathBuf::from(path!("/code/project2")),
+    )
+    .await;
 
     let (project, _headless) = init_test(&fs, cx, server_cx).await;
 
@@ -2903,6 +2911,152 @@ async fn test_remote_resolve_abs_path(cx: &mut TestAppContext, server_cx: &mut T
         })
         .await;
     assert!(path.is_none());
+
+    for path in [
+        path!("/code/project1/../project2/README.md"),
+        path!("/code/project1/linked/README.md"),
+    ] {
+        let resolved_path = project
+            .update(cx, |project, cx| project.resolve_abs_path(path, cx))
+            .await
+            .expect("existing path should resolve without canonicalization");
+        assert_eq!(resolved_path.abs_path(), Some(path));
+        let resolved_file_path = project
+            .update(cx, |project, cx| project.resolve_abs_file_path(path, cx))
+            .await
+            .expect("existing file path should resolve without canonicalization");
+        assert_eq!(resolved_file_path.abs_path(), Some(path));
+    }
+}
+
+#[gpui::test]
+async fn test_resolve_abs_file_path_canonical(
+    cx: &mut TestAppContext,
+    server_cx: &mut TestAppContext,
+) {
+    let fs = FakeFs::new(server_cx.executor());
+    fs.insert_tree(
+        path!("/code"),
+        json!({
+            "project1": {},
+            "project2": {"README.md": "# project 2"},
+        }),
+    )
+    .await;
+    fs.insert_symlink(
+        path!("/code/project1/linked"),
+        PathBuf::from(path!("/code/project2")),
+    )
+    .await;
+
+    let (remote_project, _headless) = init_test(&fs, cx, server_cx).await;
+    let local_project = Project::test(fs, [], cx).await;
+    for project in [remote_project, local_project] {
+        let path = path!("/code/project1/linked/README.md");
+        let resolved_path = project
+            .update(cx, |project, cx| project.resolve_abs_path(path, cx))
+            .await
+            .expect("existing path should resolve without canonicalization");
+        assert_eq!(resolved_path.abs_path(), Some(path));
+
+        for path in [
+            path!("/code/project1/linked"),
+            path!("/code/project1/linked/missing.md"),
+        ] {
+            let resolved_path = project
+                .update(cx, |project, cx| {
+                    project.resolve_abs_file_path_canonical(path, cx)
+                })
+                .await
+                .expect("missing files and directories should not cause resolution errors");
+            assert!(resolved_path.is_none(), "{path}");
+        }
+
+        let (worktree, _) = project
+            .update(cx, |project, cx| {
+                project.find_or_create_worktree(path!("/code/project2/README.md"), false, cx)
+            })
+            .await
+            .expect("canonical file worktree should open");
+        worktree.read_with(cx, |worktree, _| {
+            assert!(!worktree.is_visible());
+            assert!(worktree.is_single_file());
+        });
+
+        for path in [
+            path!("/code/project1/../project2/README.md"),
+            path!("/code/project1/linked/README.md"),
+        ] {
+            let resolved_path = project
+                .update(cx, |project, cx| {
+                    project.resolve_abs_file_path_canonical(path, cx)
+                })
+                .await
+                .expect("existing file path should canonicalize without errors")
+                .expect("existing path should resolve");
+            assert!(resolved_path.is_file());
+            assert_eq!(
+                resolved_path.abs_path(),
+                Some(path!("/code/project2/README.md")),
+                "{path}"
+            );
+            let (opened_worktree, relative_path) = project
+                .update(cx, |project, cx| {
+                    project.find_or_create_worktree(
+                        resolved_path.abs_path().expect("path should be absolute"),
+                        false,
+                        cx,
+                    )
+                })
+                .await
+                .expect("resolved file worktree should open");
+            assert!(relative_path.is_empty());
+            opened_worktree.read_with(cx, |opened_worktree, cx| {
+                assert_eq!(opened_worktree.id(), worktree.read(cx).id(), "{path}");
+                assert_eq!(project.read(cx).worktrees(cx).count(), 1);
+            });
+        }
+
+        let (project_worktree, _) = project
+            .update(cx, |project, cx| {
+                project.find_or_create_worktree(path!("/code/project1"), true, cx)
+            })
+            .await
+            .unwrap();
+        let alias = project_worktree.read_with(cx, |worktree, _| {
+            ProjectPath::from((worktree.id(), rel_path("linked/README.md")))
+        });
+        let buffer = project
+            .update(cx, |project, cx| project.open_buffer(alias.clone(), cx))
+            .await
+            .unwrap();
+        buffer.update(cx, |buffer, cx| {
+            buffer.edit([(0..0, "unsaved ")], None, cx);
+        });
+
+        let resolved = project
+            .update(cx, |project, cx| {
+                project.resolve_abs_file_link(path!("/code/project1//linked/README.md"), cx)
+            })
+            .await
+            .unwrap()
+            .unwrap();
+        assert_eq!(resolved.project_path(), Some(&alias));
+        let reopened = project
+            .update(cx, |project, cx| {
+                project.open_buffer(resolved.project_path().unwrap().clone(), cx)
+            })
+            .await
+            .unwrap();
+        assert_eq!(reopened, buffer);
+        buffer.read_with(cx, |buffer, _| {
+            assert_eq!(buffer.text(), "unsaved # project 2");
+            assert!(buffer.is_dirty());
+        });
+        project.read_with(cx, |project, cx| {
+            assert_eq!(project.worktrees(cx).count(), 2);
+        });
+    }
 }
 
 #[gpui::test(iterations = 10)]

crates/util/src/paths.rs

@@ -632,17 +632,17 @@ impl PathWithPosition {
             LazyLock::new(|| Regex::new(ROW_COL_CAPTURE_REGEX).unwrap());
         match SUFFIX_RE
             .captures(maybe_file_name_with_row_col)
-            .map(|caps| caps.extract())
-        {
-            Some((_, [file_name, maybe_row, maybe_column])) => {
+            .and_then(|captures| {
+                let file_name_end = captures.iter().skip(1).flatten().next()?.end();
+                let (_, [_, row, column]) = captures.extract();
+                Some((file_name_end, row, column))
+            }) {
+            Some((file_name_end, maybe_row, maybe_column)) => {
                 let row = maybe_row.parse::<u32>().ok();
                 let column = maybe_column.parse::<u32>().ok();
 
-                let (_, suffix) = trimmed.split_once(file_name).unwrap();
-                let path_without_suffix = &trimmed[..trimmed.len() - suffix.len()];
-
                 Self {
-                    path: Path::new(path_without_suffix).to_path_buf(),
+                    path: path.with_file_name(&maybe_file_name_with_row_col[..file_name_end]),
                     row,
                     column,
                 }
@@ -651,8 +651,12 @@ impl PathWithPosition {
                 // The `ROW_COL_CAPTURE_REGEX` deals with separated digits only,
                 // but in reality there could be `foo/bar.py:22:in` inputs which we want to match too.
                 // The regex mentioned is not very extendable with "digit or random string" checks, so do this here instead.
+                let path = Path::new(s);
+                let Some(file_name) = path.file_name().and_then(|name| name.to_str()) else {
+                    return Self::from_path(path.to_path_buf());
+                };
                 let delimiter = ':';
-                let mut path_parts = s
+                let mut path_parts = file_name
                     .rsplitn(3, delimiter)
                     .collect::<Vec<_>>()
                     .into_iter()
@@ -681,7 +685,11 @@ impl PathWithPosition {
                 }
 
                 Self {
-                    path: PathBuf::from(path_string),
+                    path: if row.is_some() {
+                        path.with_file_name(path_string)
+                    } else {
+                        path.to_path_buf()
+                    },
                     row,
                     column,
                 }
@@ -2336,6 +2344,74 @@ mod tests {
         );
     }
 
+    #[test]
+    fn path_with_position_preserves_paths_without_positions() {
+        for input in [
+            "",
+            "/",
+            "thf/",
+            "/test/cool/",
+            "/test/Ῥόδος/",
+            "parent//file.rs",
+            "parent/./file.rs",
+            "parent/file.rs/.",
+            "parent:12:3/file.rs/",
+            "file.rs:invalid/",
+            " parent/file.rs ",
+            r"C:\test\cool\",
+            r"C:\test\Ῥόδος\",
+            r"\\server\share\cool\",
+        ] {
+            let parsed = PathWithPosition::parse_str(input);
+            assert_eq!(parsed.path.as_os_str(), Path::new(input).as_os_str());
+            assert_eq!(parsed.row, None, "{input}");
+            assert_eq!(parsed.column, None, "{input}");
+        }
+    }
+
+    #[test]
+    fn path_with_position_preserves_repeated_filename() {
+        for (input, expected_path) in [
+            ("file.rs/file.rs:12:3", "file.rs/file.rs"),
+            ("file.rs-backups/file.rs(12,3)", "file.rs-backups/file.rs"),
+            ("😀.rs/😀.rs:12:3", "😀.rs/😀.rs"),
+            ("parent/a(b).rs(12,3)", "parent/a(b).rs"),
+            ("parent/😀(b).rs(12,3)", "parent/😀(b).rs"),
+            ("parent/a(b).rs:(12,3)", "parent/a(b).rs"),
+            (r"src\file.rs\file.rs:12:3", r"src\file.rs\file.rs"),
+        ] {
+            assert_eq!(
+                PathWithPosition::parse_str(input),
+                PathWithPosition {
+                    path: PathBuf::from(expected_path),
+                    row: Some(12),
+                    column: Some(3),
+                },
+                "{input}",
+            );
+        }
+    }
+
+    #[test]
+    fn path_with_position_preserves_parent_position_like_components() {
+        for (input, expected_path, row) in [
+            ("parent:12:3/file.rs", "parent:12:3/file.rs", None),
+            ("parent:12:3/file.rs:4:in", "parent:12:3/file.rs", Some(4)),
+            (" parent/file.rs ", " parent/file.rs ", None),
+            (" parent/file.rs:4:in ", " parent/file.rs", Some(4)),
+        ] {
+            assert_eq!(
+                PathWithPosition::parse_str(input),
+                PathWithPosition {
+                    path: PathBuf::from(expected_path),
+                    row,
+                    column: None,
+                },
+                "{input}",
+            );
+        }
+    }
+
     #[perf]
     fn path_with_position_parse_posix_path() {
         // Test POSIX filename edge cases