From 5688167d224b5eca54875d49afb8bfd73a07915a Mon Sep 17 00:00:00 2001 From: saberoueslati Date: Tue, 14 Apr 2026 10:27:37 +0100 Subject: [PATCH] recent_projects: Fix worktree path resolving to bare repo in recent projects modal (#52996) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Context When using a bare-repo-based git worktree layout (e.g. `foo/.bare` as the bare repository and `foo/my-feature` as a linked worktree), the "recent projects" modal was showing `foo/.bare` instead of `foo/my-feature`. The root cause was in `original_repo_path_from_common_dir` — when resolving a linked worktree back to its "main" repo, the function reads the `commondir` file which, for bare repos, points to the bare directory itself (e.g. `foo/.bare`). Since that path doesn't end in `.git`, the old code fell back to returning it as-is. This bare repo path was then substituted into the workspace's stored paths, causing the modal to show the bare directory instead of the actual worktree the user had opened. The fix makes `original_repo_path_from_common_dir` return `Option`, returning `None` when `common_dir` doesn't end with `.git`. `resolve_git_worktree_to_main_repo` propagates this `None`, meaning the original worktree path is preserved in recent projects rather than being replaced with the bare repo path. Closes #52931 Video of the manual test after the fix is below : [Screencast from 2026-04-02 16-05-48.webm](https://github.com/user-attachments/assets/9659c7a7-c095-4c23-af59-17715f84ce3e) ## How to Review - **`crates/git/src/repository.rs`** : `original_repo_path_from_common_dir` changed to return `Option`. Returns `None` for bare repos (no `.git` suffix). The existing `original_repo_path` call site falls back to `work_directory` when `None` is returned, preserving its prior behaviour. Unit test expectations updated accordingly, with the bare-repo case now asserting `None`. - **`crates/project/src/git_store.rs`** : `resolve_git_worktree_to_main_repo` now simply forwards the `Option` returned by `original_repo_path_from_common_dir` directly, propagating `None` for bare repos so the caller keeps the original worktree path. - **`crates/workspace/src/persistence.rs`** : New test `test_resolve_worktree_workspaces_bare_repo` exercises the exact scenario from the issue: a workspace entry pointing to a linked worktree whose `commondir` resolves to a bare repo. Asserts the path is left unchanged. ## Self-Review Checklist - [x] I've reviewed my own diff for quality, security, and reliability - [ ] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the UI/UX checklist - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Release Notes: - Fixed recent projects modal showing `.bare` folder instead of the worktree directory for bare-repo-based git worktree setups --- crates/git/src/repository.rs | 24 +++++++-------- crates/project/src/git_store.rs | 4 +-- crates/workspace/src/persistence.rs | 45 +++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 16 deletions(-) diff --git a/crates/git/src/repository.rs b/crates/git/src/repository.rs index 809081d4f0b6ab53be300fa9b04c537cb47d5096..6c3b33e94bb9b0f181eab5bb3a2690107505e143 100644 --- a/crates/git/src/repository.rs +++ b/crates/git/src/repository.rs @@ -74,6 +74,7 @@ pub fn original_repo_path( ) -> PathBuf { if common_dir != repository_dir { original_repo_path_from_common_dir(common_dir) + .unwrap_or_else(|| work_directory.to_path_buf()) } else { work_directory.to_path_buf() } @@ -86,16 +87,13 @@ pub fn original_repo_path( /// is the working directory. For a git worktree, `common_dir` is the **main** /// repo's `.git` directory, so the parent is the original repo's working directory. /// -/// Falls back to returning `common_dir` itself if it doesn't end with `.git` -/// (e.g. bare repos or unusual layouts). -pub fn original_repo_path_from_common_dir(common_dir: &Path) -> PathBuf { +/// Returns `None` if `common_dir` doesn't end with `.git` (e.g. bare repos), +/// because there is no working-tree root to resolve to in that case. +pub fn original_repo_path_from_common_dir(common_dir: &Path) -> Option { if common_dir.file_name() == Some(OsStr::new(".git")) { - common_dir - .parent() - .map(|p| p.to_path_buf()) - .unwrap_or_else(|| common_dir.to_path_buf()) + common_dir.parent().map(|p| p.to_path_buf()) } else { - common_dir.to_path_buf() + None } } @@ -4446,26 +4444,26 @@ mod tests { // Normal repo: common_dir is /.git assert_eq!( original_repo_path_from_common_dir(Path::new("/code/zed5/.git")), - PathBuf::from("/code/zed5") + Some(PathBuf::from("/code/zed5")) ); // Worktree: common_dir is the main repo's .git // (same result — that's the point, it always traces back to the original) assert_eq!( original_repo_path_from_common_dir(Path::new("/code/zed5/.git")), - PathBuf::from("/code/zed5") + Some(PathBuf::from("/code/zed5")) ); - // Bare repo: no .git suffix, returns as-is + // Bare repo: no .git suffix, returns None (no working-tree root) assert_eq!( original_repo_path_from_common_dir(Path::new("/code/zed5.git")), - PathBuf::from("/code/zed5.git") + None ); // Root-level .git directory assert_eq!( original_repo_path_from_common_dir(Path::new("/.git")), - PathBuf::from("/") + Some(PathBuf::from("/")) ); } diff --git a/crates/project/src/git_store.rs b/crates/project/src/git_store.rs index 27c84239816c905c87b37e6e5fc6a765ffa96150..ef81f8030eb87366738db55850e401bbaa3d716f 100644 --- a/crates/project/src/git_store.rs +++ b/crates/project/src/git_store.rs @@ -7182,9 +7182,7 @@ pub async fn resolve_git_worktree_to_main_repo(fs: &dyn Fs, path: &Path) -> Opti .canonicalize(&gitdir_abs.join(commondir_content.trim())) .await .ok()?; - Some(git::repository::original_repo_path_from_common_dir( - &common_dir, - )) + git::repository::original_repo_path_from_common_dir(&common_dir) } /// Validates that the resolved worktree directory is acceptable: diff --git a/crates/workspace/src/persistence.rs b/crates/workspace/src/persistence.rs index 92be09286956ab33202391f56a1dec8ef83d0f92..48e94f2689b43d993d0ca9fc83e8dab9edd9c73d 100644 --- a/crates/workspace/src/persistence.rs +++ b/crates/workspace/src/persistence.rs @@ -4666,6 +4666,51 @@ mod tests { assert_eq!(result[2].0, WorkspaceId(4)); } + #[gpui::test] + async fn test_resolve_worktree_workspaces_bare_repo(cx: &mut gpui::TestAppContext) { + let fs = fs::FakeFs::new(cx.executor()); + + // Bare repo at /foo/.bare (commondir doesn't end with .git) + fs.insert_tree( + "/foo/.bare", + json!({ + "worktrees": { + "my-feature": { + "commondir": "../../", + "HEAD": "ref: refs/heads/my-feature" + } + } + }), + ) + .await; + + // Linked worktree whose commondir resolves to a bare repo (/foo/.bare) + fs.insert_tree( + "/foo/my-feature", + json!({ + ".git": "gitdir: /foo/.bare/worktrees/my-feature", + "src": { "main.rs": "" } + }), + ) + .await; + + let t0 = Utc::now(); + + let workspaces = vec![( + WorkspaceId(1), + SerializedWorkspaceLocation::Local, + PathList::new(&["/foo/my-feature"]), + t0, + )]; + + let result = resolve_worktree_workspaces(workspaces, fs.as_ref()).await; + + // The worktree path must be preserved unchanged — /foo/.bare is a bare repo + // and cannot serve as a working-tree root, so resolution must return None. + assert_eq!(result.len(), 1); + assert_eq!(result[0].2.paths(), &[PathBuf::from("/foo/my-feature")]); + } + #[gpui::test] async fn test_restore_window_with_linked_worktree_and_multiple_project_groups( cx: &mut gpui::TestAppContext,