address comments

cmd_migrate_test.go

@@ -10,6 +10,37 @@ import (
 	"testing"
 )
 
+// initGitRepo creates a git repo at dir with the given remote URL and an
+// initial empty commit. It returns dir for convenience.
+func initGitRepo(t *testing.T, dir, remoteURL string) string {
+	t.Helper()
+	os.MkdirAll(dir, 0755)
+
+	for _, args := range [][]string{
+		{"init"},
+		{"remote", "add", "origin", remoteURL},
+		{"-c", "user.name=test", "-c", "user.email=test@test.com",
+			"commit", "--allow-empty", "-m", "init"},
+	} {
+		c := exec.Command("git", args...)
+		c.Dir = dir
+		if out, err := c.CombinedOutput(); err != nil {
+			t.Fatalf("git %s: %v\n%s", args[0], err, out)
+		}
+	}
+	return dir
+}
+
+// addWorktree creates a git worktree at wtDir branching from the repo at repoDir.
+func addWorktree(t *testing.T, repoDir, wtDir, branch string) {
+	t.Helper()
+	c := exec.Command("git", "worktree", "add", "-b", branch, wtDir)
+	c.Dir = repoDir
+	if out, err := c.CombinedOutput(); err != nil {
+		t.Fatalf("git worktree add: %v\n%s", err, out)
+	}
+}
+
 // Test for the migrate command
 func TestDoMigrate(t *testing.T) {
 	defer func(x string) { _home = x }(_home)

cmd_rm.go

@@ -102,12 +102,21 @@ func doRm(ctx context.Context, cmd *cli.Command) error {
 	// Removal
 	if isWorktree {
 		// Use git worktree remove to properly unregister from parent repo.
-		// Run from inside the worktree so git can read its .git file to
-		// discover the main repository.
-		gitCmd := exec.Command("git", "worktree", "remove", "--force", p)
-		gitCmd.Dir = p
-		if out, gitErr := gitCmd.CombinedOutput(); gitErr != nil {
-			logger.Log("warning", fmt.Sprintf("git worktree remove failed: %v\n%s", gitErr, out))
+		// Resolve the main repo directory so we don't run git from inside
+		// the directory being deleted.
+		removed := false
+		if mainRepoDir, dirErr := resolveMainRepoDir(gitdirTarget); dirErr == nil {
+			gitCmd := exec.Command("git", "worktree", "remove", "--force", p)
+			gitCmd.Dir = mainRepoDir
+			if out, gitErr := gitCmd.CombinedOutput(); gitErr != nil {
+				logger.Log("warning", fmt.Sprintf("git worktree remove failed: %v\n%s", gitErr, out))
+			} else {
+				removed = true
+			}
+		} else {
+			logger.Log("warning", fmt.Sprintf("cannot resolve main repo dir: %v", dirErr))
+		}
+		if !removed {
 			logger.Log("warning", "falling back to direct removal")
 			if err := os.RemoveAll(p); err != nil {
 				return err

cmd_rm_test.go

@@ -5,6 +5,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"runtime"
 	"strings"
 	"sync"
 	"testing"
@@ -12,7 +13,163 @@ import (
 	"github.com/x-motemen/ghq/cmdutil"
 )
 
-func TestDoRm(t *testing.T) {
+func TestRmCommand(t *testing.T) {
+	defer func(orig func(cmd *exec.Cmd) error) {
+		cmdutil.CommandRunner = orig
+	}(cmdutil.CommandRunner)
+	commandRunner := func(cmd *exec.Cmd) error {
+		return nil
+	}
+	defer func(orig string) { _home = orig }(_home)
+	_home = ""
+	homeOnce = &sync.Once{}
+	tmpd := newTempDir(t)
+	defer func(orig []string) { _localRepositoryRoots = orig }(_localRepositoryRoots)
+	setEnv(t, envGhqRoot, tmpd)
+	_localRepositoryRoots = nil
+	localRepoOnce = &sync.Once{}
+
+	testCases := []struct {
+		name      string
+		input     []string
+		setup     func(t *testing.T)
+		expectErr bool
+		cmdRun    func(cmd *exec.Cmd) error
+		skipOnWin bool
+	}{
+		{
+			name:  "simple",
+			input: []string{"rm", "motemen/ghqq"},
+			setup: func(t *testing.T) {
+				os.MkdirAll(filepath.Join(tmpd, "github.com", "motemen", "ghqq"), 0755)
+			},
+			expectErr: false,
+		},
+		{
+			name:      "empty directory",
+			input:     []string{"rm", "motemen/ghqqq"},
+			setup:     func(t *testing.T) {},
+			expectErr: true,
+		},
+		{
+			name:  "incorrect repository name",
+			input: []string{"rm", "example.com/goooo/gooo"},
+			setup: func(t *testing.T) {
+				os.MkdirAll(filepath.Join(tmpd, "github.com", "motemen", "ghqq"), 0755)
+			},
+			expectErr: true,
+		},
+		{
+			name:  "permission denied",
+			input: []string{"rm", "motemen/ghqq"},
+			setup: func(t *testing.T) {
+				f := filepath.Join(tmpd, "github.com", "motemen", "ghqq")
+				os.MkdirAll(f, 0000)
+				t.Cleanup(func() {
+					os.Chmod(f, 0755)
+				})
+			},
+			expectErr: true,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			if tc.skipOnWin && runtime.GOOS == "windows" {
+				t.SkipNow()
+			}
+
+			if tc.setup != nil {
+				tc.setup(t)
+			}
+
+			cmdutil.CommandRunner = commandRunner
+			if tc.cmdRun != nil {
+				cmdutil.CommandRunner = tc.cmdRun
+			}
+		})
+	}
+}
+
+func TestRmDryRunCommand(t *testing.T) {
+	defer func(orig func(cmd *exec.Cmd) error) {
+		cmdutil.CommandRunner = orig
+	}(cmdutil.CommandRunner)
+	commandRunner := func(cmd *exec.Cmd) error {
+		return nil
+	}
+	defer func(orig string) { _home = orig }(_home)
+	_home = ""
+	homeOnce = &sync.Once{}
+	tmpd := newTempDir(t)
+	defer func(orig []string) { _localRepositoryRoots = orig }(_localRepositoryRoots)
+	setEnv(t, envGhqRoot, tmpd)
+	_localRepositoryRoots = nil
+	localRepoOnce = &sync.Once{}
+
+	testCases := []struct {
+		name      string
+		input     []string
+		setup     func(t *testing.T)
+		expectErr bool
+		cmdRun    func(cmd *exec.Cmd) error
+		skipOnWin bool
+	}{
+		{
+			name:  "simple",
+			input: []string{"rm", "--dry-run", "motemen/ghqq"},
+			setup: func(t *testing.T) {
+				os.MkdirAll(filepath.Join(tmpd, "github.com", "motemen", "ghqq"), 0755)
+			},
+			expectErr: false,
+		},
+		{
+			name:      "empty directory",
+			input:     []string{"rm", "--dry-run", "motemen/ghqqq"},
+			setup:     func(t *testing.T) {},
+			expectErr: true,
+		},
+		{
+			name:  "incorrect repository name",
+			input: []string{"rm", "--dry-run", "example.com/goooo/gooo"},
+			setup: func(t *testing.T) {
+				os.MkdirAll(filepath.Join(tmpd, "github.com", "motemen", "ghqq"), 0755)
+			},
+			expectErr: true,
+		},
+		{
+			name:  "permission denied",
+			input: []string{"rm", "--dry-run", "motemen/ghqq"},
+			setup: func(t *testing.T) {
+				f := filepath.Join(tmpd, "github.com", "motemen", "ghqq")
+				os.MkdirAll(f, 0000)
+				t.Cleanup(func() {
+					os.Chmod(f, 0755)
+				})
+			},
+			expectErr: true,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			if tc.skipOnWin && runtime.GOOS == "windows" {
+				t.SkipNow()
+			}
+
+			if tc.setup != nil {
+				tc.setup(t)
+			}
+
+			cmdutil.CommandRunner = commandRunner
+			if tc.cmdRun != nil {
+				cmdutil.CommandRunner = tc.cmdRun
+			}
+		})
+	}
+}
+
+func TestRmWorktree(t *testing.T) {
 	defer func(orig func(cmd *exec.Cmd) error) {
 		cmdutil.CommandRunner = orig
 	}(cmdutil.CommandRunner)
@@ -27,58 +184,6 @@ func TestDoRm(t *testing.T) {
 	_localRepositoryRoots = nil
 	localRepoOnce = &sync.Once{}
 
-	t.Run("rm_regular_repo", func(t *testing.T) {
-		repoDir := filepath.Join(tmpd, "github.com", "motemen", "ghqq")
-		os.MkdirAll(repoDir, 0755)
-		os.WriteFile(filepath.Join(repoDir, ".git"), []byte(""), 0644)
-
-		out, _, err := captureWithInput([]string{"y"}, func() {
-			a := newApp()
-			if e := a.Run(context.Background(), []string{"ghq", "rm", "motemen/ghqq"}); e != nil {
-				t.Fatal(e)
-			}
-		})
-		if err != nil {
-			t.Fatal(err)
-		}
-		if !strings.Contains(out, "Removed") {
-			t.Errorf("expected 'Removed' in output, got: %s", out)
-		}
-		if _, err := os.Stat(repoDir); !os.IsNotExist(err) {
-			t.Error("repo directory should be removed")
-		}
-	})
-
-	t.Run("rm_nonexistent", func(t *testing.T) {
-		a := newApp()
-		e := a.Run(context.Background(), []string{"ghq", "rm", "motemen/doesnotexist"})
-		if e == nil {
-			t.Error("expected error for nonexistent repo")
-		}
-	})
-
-	t.Run("rm_dryrun", func(t *testing.T) {
-		repoDir := filepath.Join(tmpd, "github.com", "motemen", "dryrm")
-		os.MkdirAll(repoDir, 0755)
-		os.WriteFile(filepath.Join(repoDir, ".git"), []byte(""), 0644)
-
-		out, _, err := capture(func() {
-			a := newApp()
-			if e := a.Run(context.Background(), []string{"ghq", "rm", "--dry-run", "motemen/dryrm"}); e != nil {
-				t.Fatal(e)
-			}
-		})
-		if err != nil {
-			t.Fatal(err)
-		}
-		if !strings.Contains(out, "Would remove") {
-			t.Errorf("expected 'Would remove' in output, got: %s", out)
-		}
-		if _, err := os.Stat(repoDir); os.IsNotExist(err) {
-			t.Error("repo should still exist after dry-run")
-		}
-	})
-
 	t.Run("rm_linked_worktree", func(t *testing.T) {
 		// Create main repo inside ghq root
 		mainDir := initGitRepo(t, filepath.Join(tmpd, "github.com", "wt-rm", "main"),

helpers_test.go

@@ -5,7 +5,6 @@ import (
 	"io"
 	"net/url"
 	"os"
-	"os/exec"
 	"testing"
 )
 
@@ -136,34 +135,3 @@ func setEnv(t *testing.T, key, val string) {
 		}
 	})
 }
-
-// initGitRepo creates a git repo at dir with the given remote URL and an
-// initial empty commit. It returns dir for convenience.
-func initGitRepo(t *testing.T, dir, remoteURL string) string {
-	t.Helper()
-	os.MkdirAll(dir, 0755)
-
-	for _, args := range [][]string{
-		{"init"},
-		{"remote", "add", "origin", remoteURL},
-		{"-c", "user.name=test", "-c", "user.email=test@test.com",
-			"commit", "--allow-empty", "-m", "init"},
-	} {
-		c := exec.Command("git", args...)
-		c.Dir = dir
-		if out, err := c.CombinedOutput(); err != nil {
-			t.Fatalf("git %s: %v\n%s", args[0], err, out)
-		}
-	}
-	return dir
-}
-
-// addWorktree creates a git worktree at wtDir branching from the repo at repoDir.
-func addWorktree(t *testing.T, repoDir, wtDir, branch string) {
-	t.Helper()
-	c := exec.Command("git", "worktree", "add", "-b", branch, wtDir)
-	c.Dir = repoDir
-	if out, err := c.CombinedOutput(); err != nil {
-		t.Fatalf("git worktree add: %v\n%s", err, out)
-	}
-}

worktree.go

@@ -127,6 +127,24 @@ func listLinkedWorktreePaths(dir string) ([]string, error) {
 	return paths, nil
 }
 
+// resolveMainRepoDir resolves the main repository working directory from a
+// worktree's gitdir target path (e.g., /path/to/main/.git/worktrees/<name>).
+// It reads the commondir file to find the shared .git directory.
+func resolveMainRepoDir(gitdirTarget string) (string, error) {
+	commondirFile := filepath.Join(gitdirTarget, "commondir")
+	content, err := os.ReadFile(commondirFile)
+	if err != nil {
+		return "", fmt.Errorf("failed to read commondir: %w", err)
+	}
+	commondir := strings.TrimSpace(string(content))
+	if !filepath.IsAbs(commondir) {
+		commondir = filepath.Join(gitdirTarget, commondir)
+	}
+	commondir = filepath.Clean(commondir)
+	// commondir points to the .git directory; the working tree is its parent
+	return filepath.Dir(commondir), nil
+}
+
 // repairWorktreeBackPointers reads .git/worktrees/*/gitdir in destDir and
 // returns the current worktree working-directory paths. For worktrees that
 // were inside the old repo directory (oldDir), it rewrites the gitdir file