diff --git a/README.md b/README.md index a0a0590..efc19bb 100644 --- a/README.md +++ b/README.md @@ -174,8 +174,10 @@ agentws new argocd-apps --branch benvin/hotfix --from release-1.2 # List managed worktrees (repo, branch, path) agentws list -# Remove a worktree (by path or branch); refreshes the source repo afterwards +# Remove a worktree (by path or branch); refreshes the source repo afterwards. +# A branch name shared by several repos is refused; pass a path or --repo. agentws rm benvin/my-change +agentws rm benvin/my-change --repo argocd-apps agentws rm ~/.cache/agentws/argocd-apps__benvin-my-change --delete-branch # Classify every worktree found; dry run unless --yes is given diff --git a/cmd/agentws/main.go b/cmd/agentws/main.go index f15bd5f..e075ad2 100644 --- a/cmd/agentws/main.go +++ b/cmd/agentws/main.go @@ -12,7 +12,7 @@ // // agentws new [--branch benvin/] [--from ] // agentws list -// agentws rm [--delete-branch] +// agentws rm [--repo ] [--delete-branch] // agentws prune [--yes] [--keep-branches] [--no-fetch] [--json] // [--include-unmanaged] [--include-keep] // agentws clean @@ -591,6 +591,7 @@ func underRoot(path, root string) bool { func newRmCmd() *cobra.Command { var deleteBranch bool + var repo string cmd := &cobra.Command{ Use: "rm ", Short: "Remove a managed worktree and refresh its source repo", @@ -598,7 +599,7 @@ func newRmCmd() *cobra.Command { SilenceUsage: true, RunE: func(cmd *cobra.Command, args []string) error { target := strings.TrimSpace(args[0]) - wt, err := resolveWorktree(target) + wt, err := resolveWorktree(target, repo) if err != nil { return err } @@ -607,22 +608,41 @@ func newRmCmd() *cobra.Command { }, } cmd.Flags().BoolVar(&deleteBranch, "delete-branch", false, "Also delete the local branch after removing the worktree") + cmd.Flags().StringVar(&repo, "repo", "", "Only match worktrees of this repo (disambiguates a branch name)") return cmd } -// resolveWorktree finds a managed worktree by exact path or by branch name. -func resolveWorktree(target string) (managedWt, error) { +// resolveWorktree finds a managed worktree by exact path or by branch name. A +// branch name shared by several repos is refused unless repo narrows it to one. +func resolveWorktree(target, repo string) (managedWt, error) { managed, err := managedWorktrees() if err != nil { return managedWt{}, err } abs, _ := filepath.Abs(target) + var matches []managedWt for _, w := range managed { - if w.path == target || w.path == abs || (w.branch != "" && w.branch == target) { + if repo != "" && w.repo != repo { + continue + } + if w.path == target || w.path == abs { return w, nil } + if w.branch != "" && w.branch == target { + matches = append(matches, w) + } } - return managedWt{}, fmt.Errorf("no managed worktree matching %q (try `agentws list`)", target) + switch len(matches) { + case 0: + return managedWt{}, fmt.Errorf("no managed worktree matching %q (try `agentws list`)", target) + case 1: + return matches[0], nil + } + paths := make([]string, len(matches)) + for i, w := range matches { + paths[i] = " " + w.path + } + return managedWt{}, fmt.Errorf("branch %q matches %d worktrees; pass a path or --repo:\n%s", target, len(matches), strings.Join(paths, "\n")) } // removeWorktree removes a managed worktree and, when asked, its local branch. diff --git a/cmd/agentws/rm_test.go b/cmd/agentws/rm_test.go new file mode 100644 index 0000000..c54e985 --- /dev/null +++ b/cmd/agentws/rm_test.go @@ -0,0 +1,103 @@ +package main + +import ( + "path/filepath" + "strings" + "testing" + + "git.unkin.net/unkin/agent-tools/internal/agent" +) + +// addOtherRepoWorktree clones a second source repo "other" and gives it a +// managed worktree on branch, so two repos share one branch name. +func addOtherRepoWorktree(t *testing.T, f *fixture, branch string) string { + t.Helper() + src := filepath.Join(f.root, "src", "other") + git(t, filepath.Join(f.root, "src"), "clone", f.bare, src) + path := filepath.Join(f.wtRoot, agent.WorktreeDirName("other", branch)) + git(t, src, "worktree", "add", path, "-b", branch, "origin/main") + return path +} + +func TestResolveUniqueBranch(t *testing.T) { + f := newFixture(t) + want := f.addWorktree(t, "benvin/one") + f.addWorktree(t, "benvin/two") + wt, err := resolveWorktree("benvin/one", "") + if err != nil { + t.Fatal(err) + } + if wt.path != want { + t.Errorf("path %q, want %q", wt.path, want) + } +} + +func TestResolveAmbiguousBranchRefused(t *testing.T) { + f := newFixture(t) + a := f.addWorktree(t, "benvin/shared") + b := addOtherRepoWorktree(t, f, "benvin/shared") + _, err := resolveWorktree("benvin/shared", "") + if err == nil { + t.Fatal("ambiguous branch resolved, want refusal") + } + for _, p := range []string{a, b} { + if !strings.Contains(err.Error(), p) { + t.Errorf("error %q does not list candidate %s", err, p) + } + } +} + +func TestRmAmbiguousBranchRemovesNothing(t *testing.T) { + f := newFixture(t) + a := f.addWorktree(t, "benvin/shared") + b := addOtherRepoWorktree(t, f, "benvin/shared") + cmd := newRootCmd() + cmd.SetArgs([]string{"rm", "benvin/shared"}) + cmd.SetOut(new(strings.Builder)) + cmd.SetErr(new(strings.Builder)) + if err := cmd.Execute(); err == nil { + t.Fatal("rm succeeded on an ambiguous branch") + } + if !exists(a) || !exists(b) { + t.Error("ambiguous rm removed a worktree") + } +} + +func TestResolveByPathDespiteSharedBranch(t *testing.T) { + f := newFixture(t) + f.addWorktree(t, "benvin/shared") + b := addOtherRepoWorktree(t, f, "benvin/shared") + wt, err := resolveWorktree(b, "") + if err != nil { + t.Fatal(err) + } + if wt.path != b || wt.repo != "other" { + t.Errorf("got %s (%s), want %s (other)", wt.path, wt.repo, b) + } +} + +func TestResolveRepoDisambiguates(t *testing.T) { + f := newFixture(t) + a := f.addWorktree(t, "benvin/shared") + b := addOtherRepoWorktree(t, f, "benvin/shared") + for repo, want := range map[string]string{"repo": a, "other": b} { + wt, err := resolveWorktree("benvin/shared", repo) + if err != nil { + t.Fatalf("--repo %s: %v", repo, err) + } + if wt.path != want { + t.Errorf("--repo %s: path %q, want %q", repo, wt.path, want) + } + } + if _, err := resolveWorktree("benvin/shared", "missing"); err == nil { + t.Error("--repo missing resolved, want no match") + } +} + +func TestResolvePathOutsideRepoFilterRefused(t *testing.T) { + f := newFixture(t) + a := f.addWorktree(t, "benvin/one") + if _, err := resolveWorktree(a, "other"); err == nil { + t.Error("path in repo resolved under --repo other") + } +}