fix: improve monorepo & worktree interaction (#33)#34
Conversation
Detect the actual git repository root via `git rev-parse --show-toplevel` instead of assuming the project path is the repo root. In monorepo setups where the project is a subdirectory of the git root, worktrees are now created at the correct location. Before: /projects/monorepo/app-wt/branch After: /projects/monorepo-wt/branch/app - Add `get_repo_root()` to git module - Store git root and subdir in WorktreeDialog for correct path computation - Add `worktree_path` field to WorktreeMetadata for correct removal - Extract path computation into testable `compute_target_paths()` function - Add 5 unit tests covering simple repos, monorepos, nested subdirs, absolute templates, and branch name sanitization
JanTvrdik
changed the title
JanTvrdik
force-pushed
the
fix/monorepo-worktree-and-fresh-install-persistence
branch
from
3718fc9 to
46e88fe
Compare
There was a problem hiding this comment.
Pull request overview
This PR fixes git worktree path computation in monorepo setups by basing worktree creation on the actual git repository root (via git rev-parse --show-toplevel) rather than assuming the project directory is the repo root. It also fixes first-run workspace persistence by only disabling auto-save when a real backup exists, avoiding the “fresh install can’t save” scenario.
Changes:
- Add
git::get_repo_root()and use it to compute correct repo-root-relative worktree locations for monorepos. - Persist worktree checkout path separately from project path (
WorktreeMetadata.worktree_path) and use it for reliable worktree removal/back-compat. - Adjust workspace load behavior so auto-save is only disabled when
workspace.json.bakexists and recovery failed.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/workspace/state.rs |
Extends persisted worktree metadata to include a dedicated worktree_path for monorepo correctness. |
src/workspace/persistence.rs |
Refines “disable auto-save” logic to allow saving on fresh installs while still protecting recoverable backups. |
src/workspace/actions/project.rs |
Updates worktree creation/removal to use git repo root + stored worktree checkout path. |
src/views/overlays/worktree_dialog.rs |
Computes both worktree checkout path and project path (subdir-aware) and adds unit tests for path computation. |
src/git/repository.rs |
Introduces get_repo_root() implementation and a basic unit test for invalid paths. |
src/git/mod.rs |
Re-exports get_repo_root. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let (_, path) = this.get_target_paths(&branch); | ||
| this.custom_path_input.update(cx, |input, cx| { | ||
| input.set_value(&path, cx); |
There was a problem hiding this comment.
When enabling “Use custom path”, the input is pre-filled with project_path (the second value from get_target_paths). In monorepo cases this includes the subdir, but later create_worktree treats the custom input as the worktree checkout path and appends subdir again, producing a duplicated subdirectory (e.g. /wt/branch/app/app). Pre-fill should use the computed worktree_path (first tuple element), or alternatively treat the custom input consistently as the project path and avoid re-appending subdir.
| let (_, path) = this.get_target_paths(&branch); | |
| this.custom_path_input.update(cx, |input, cx| { | |
| input.set_value(&path, cx); | |
| let (worktree_path, _) = this.get_target_paths(&branch); | |
| this.custom_path_input.update(cx, |input, cx| { | |
| input.set_value(&worktree_path, cx); |
| let project_pathbuf = PathBuf::from(&project_path); | ||
| let git_root = git::get_repo_root(&project_pathbuf) | ||
| .unwrap_or_else(|| project_pathbuf.clone()); | ||
| let subdir = project_pathbuf.strip_prefix(&git_root) | ||
| .unwrap_or(Path::new("")) | ||
| .to_path_buf(); |
There was a problem hiding this comment.
subdir is computed via project_pathbuf.strip_prefix(&git_root) and falls back to "" if it fails. This can silently break monorepo detection when project_path is relative or otherwise not byte-for-byte prefixed by git_root (common on Windows due to path casing, or when git returns an absolute toplevel). Consider normalizing/absolutizing both paths (e.g., canonicalize with a fallback) and surfacing an error/warn if strip_prefix fails instead of treating it as “repo root” (subdir = "").
| let git_root = Path::new("/projects/myrepo"); | ||
| let subdir = Path::new(""); | ||
| let (wt, proj) = compute_target_paths(git_root, subdir, "../{repo}-wt/{branch}", "feature"); | ||
| assert_eq!(wt, "/projects/myrepo-wt/feature"); | ||
| assert_eq!(proj, "/projects/myrepo-wt/feature"); |
There was a problem hiding this comment.
The new unit tests hard-code Unix-style absolute paths and assert against /... strings. These assertions will fail on Windows because path stringification uses \ separators (and /projects/... may not round-trip the same). To keep tests portable, build expected values with PathBuf joins and compare PathBufs (or normalize separators) rather than comparing raw strings.
- Fix custom path pre-fill using worktree_path instead of project_path to prevent duplicated subdirs in monorepos - Normalize paths before strip_prefix for cross-platform correctness - Make tests cross-platform by comparing Path objects instead of strings - Pass repo_path to create_worktree_project to eliminate redundant get_repo_root call and handle worktree-from-worktree correctly - Simplify map/filter/map chain in remove_worktree_project
Summary
Detect the actual git repo root via
git rev-parse --show-toplevelinstead of assuming the project path is the repo root. In monorepo setups where the project is a subdirectory, worktrees are now created at the correct location.Before:
/projects/monorepo/app-wt/branchAfter:
/projects/monorepo-wt/branch/appget_repo_root()to git moduleworktree_pathfield to WorktreeMetadata for correct removalcompute_target_paths()functionTest plan