From b130a9be5b0f0db2e41ddd82044ef6c256c93625 Mon Sep 17 00:00:00 2001 From: tidy-dev <75402236+tidy-dev@users.noreply.github.com> Date: Tue, 21 Jul 2026 20:14:47 -0400 Subject: [PATCH 01/25] Add --worktree flag to gh pr checkout Support checking out a pull request into a new git worktree via `gh pr checkout --worktree `. Re-running against the same path fast-forwards the existing worktree (idempotent, matching plain checkout), and checking out a branch already present in another worktree fails with a clear message. Adds a git.Client.Worktrees() helper that parses `git worktree list --porcelain`. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/pr/checkout/checkout.go | 171 +++++++++++++++--- pkg/cmd/pr/checkout/checkout_test.go | 252 +++++++++++++++++++++++++++ 2 files changed, 403 insertions(+), 20 deletions(-) diff --git a/pkg/cmd/pr/checkout/checkout.go b/pkg/cmd/pr/checkout/checkout.go index a137f92f50d..9dd91737afa 100644 --- a/pkg/cmd/pr/checkout/checkout.go +++ b/pkg/cmd/pr/checkout/checkout.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "net/http" + "path/filepath" "strings" "github.com/MakeNowJust/heredoc" @@ -33,6 +34,7 @@ type CheckoutOptions struct { Force bool Detach bool BranchName string + Worktree string } func NewCmdCheckout(f *cmdutil.Factory, runF func(*CheckoutOptions) error) *cobra.Command { @@ -60,6 +62,10 @@ func NewCmdCheckout(f *cmdutil.Factory, runF func(*CheckoutOptions) error) *cobr Args: cobra.MaximumNArgs(1), Aliases: []string{"co"}, RunE: func(cmd *cobra.Command, args []string) error { + if cmd.Flags().Changed("worktree") && opts.Worktree == "" { + return cmdutil.FlagErrorf("--worktree cannot be blank") + } + if len(args) > 0 { opts.PRResolver = &specificPRResolver{ prFinder: shared.NewFinder(f), @@ -97,6 +103,7 @@ func NewCmdCheckout(f *cmdutil.Factory, runF func(*CheckoutOptions) error) *cobr cmd.Flags().BoolVarP(&opts.Force, "force", "f", false, "Reset the existing local branch to the latest state of the pull request") cmd.Flags().BoolVarP(&opts.Detach, "detach", "", false, "Checkout PR with a detached HEAD") cmd.Flags().StringVarP(&opts.BranchName, "branch", "b", "", "Local branch name to use (default [the name of the head branch])") + cmd.Flags().StringVar(&opts.Worktree, "worktree", "", "Check out the pull request into a new worktree at the given `path`") return cmd } @@ -164,6 +171,13 @@ func checkoutRun(opts *CheckoutOptions) error { return err } + if opts.Worktree != "" && opts.IO.IsStdoutTTY() { + cs := opts.IO.ColorScheme() + fmt.Fprintf(opts.IO.Out, "%s Worktree ready for PR #%d\n", cs.SuccessIcon(), pr.Number) + fmt.Fprintf(opts.IO.Out, " %s\n", opts.Worktree) + fmt.Fprintf(opts.IO.Out, " To start working: cd %q\n", opts.Worktree) + } + return nil } @@ -176,24 +190,43 @@ func cmdsForExistingRemote(remote *cliContext.Remote, pr *api.PullRequest, opts refSpec += fmt.Sprintf(":refs/remotes/%s", remoteBranch) } - cmds = append(cmds, []string{"fetch", remote.Name, refSpec, "--no-tags"}) - localBranch := pr.HeadRefName if opts.BranchName != "" { localBranch = opts.BranchName } + remoteBranchRef := fmt.Sprintf("refs/remotes/%s", remoteBranch) + fetchCmd := []string{"fetch", remote.Name, refSpec, "--no-tags"} + + // FETCH_HEAD is per-worktree: when reusing an existing linked worktree in + // detach mode, fetch inside it so FETCH_HEAD is written there. + if opts.Detach && opts.Worktree != "" && isWorktreeAtPath(opts.GitClient, opts.Worktree) { + cmds = append(cmds, append([]string{"-C", opts.Worktree}, fetchCmd...)) + cmds = append(cmds, []string{"-C", opts.Worktree, "checkout", "--detach", "FETCH_HEAD"}) + return cmds + } + + cmds = append(cmds, fetchCmd) + switch { case opts.Detach: - cmds = append(cmds, []string{"checkout", "--detach", "FETCH_HEAD"}) - case localBranchExists(opts.GitClient, localBranch): - cmds = append(cmds, []string{"checkout", localBranch}) - if opts.Force { - cmds = append(cmds, []string{"reset", "--hard", fmt.Sprintf("refs/remotes/%s", remoteBranch)}) + if opts.Worktree != "" { + cmds = append(cmds, []string{"worktree", "add", "--detach", opts.Worktree, "FETCH_HEAD"}) } else { - // TODO: check if non-fast-forward and suggest to use `--force` - cmds = append(cmds, []string{"merge", "--ff-only", fmt.Sprintf("refs/remotes/%s", remoteBranch)}) + cmds = append(cmds, []string{"checkout", "--detach", "FETCH_HEAD"}) } + case opts.Worktree != "": + if isWorktreeAtPath(opts.GitClient, opts.Worktree) { + cmds = append(cmds, worktreeCheckoutCmds(opts.Worktree, localBranch, remoteBranchRef, opts.Force)...) + } else if localBranchExists(opts.GitClient, localBranch) { + cmds = append(cmds, []string{"worktree", "add", opts.Worktree, localBranch}) + cmds = append(cmds, syncBranchCmds(opts.Worktree, remoteBranchRef, opts.Force)...) + } else { + cmds = append(cmds, []string{"worktree", "add", "--track", "-b", localBranch, opts.Worktree, remoteBranch}) + } + case localBranchExists(opts.GitClient, localBranch): + cmds = append(cmds, []string{"checkout", localBranch}) + cmds = append(cmds, syncBranchCmds("", remoteBranchRef, opts.Force)...) default: cmds = append(cmds, []string{"checkout", "-b", localBranch, "--track", remoteBranch}) } @@ -206,8 +239,19 @@ func cmdsForMissingRemote(pr *api.PullRequest, baseURLOrName, repoHost, defaultB ref := fmt.Sprintf("refs/pull/%d/head", pr.Number) if opts.Detach { - cmds = append(cmds, []string{"fetch", baseURLOrName, ref, "--no-tags"}) - cmds = append(cmds, []string{"checkout", "--detach", "FETCH_HEAD"}) + fetchCmd := []string{"fetch", baseURLOrName, ref, "--no-tags"} + if opts.Worktree != "" && isWorktreeAtPath(opts.GitClient, opts.Worktree) { + // FETCH_HEAD is per-worktree; fetch inside the linked worktree. + cmds = append(cmds, append([]string{"-C", opts.Worktree}, fetchCmd...)) + cmds = append(cmds, []string{"-C", opts.Worktree, "checkout", "--detach", "FETCH_HEAD"}) + } else { + cmds = append(cmds, fetchCmd) + if opts.Worktree != "" { + cmds = append(cmds, []string{"worktree", "add", "--detach", opts.Worktree, "FETCH_HEAD"}) + } else { + cmds = append(cmds, []string{"checkout", "--detach", "FETCH_HEAD"}) + } + } return cmds } @@ -220,15 +264,29 @@ func cmdsForMissingRemote(pr *api.PullRequest, baseURLOrName, repoHost, defaultB } currentBranch, _ := opts.Branch() - if localBranch == currentBranch { - // PR head matches currently checked out branch - cmds = append(cmds, []string{"fetch", baseURLOrName, ref, "--no-tags"}) - if opts.Force { - cmds = append(cmds, []string{"reset", "--hard", "FETCH_HEAD"}) + if opts.Worktree != "" { + if isWorktreeAtPath(opts.GitClient, opts.Worktree) { + // FETCH_HEAD is per-worktree; fetch inside the linked worktree + // rather than the main worktree. We fetch to FETCH_HEAD because + // git refuses to update a branch via refspec when it is checked + // out in a worktree. + cmds = append(cmds, []string{"-C", opts.Worktree, "fetch", baseURLOrName, ref, "--no-tags"}) + // Use checkout -B to create-or-reset the branch from FETCH_HEAD. + // The local branch may not exist yet (e.g. switching the worktree + // to a different fork PR). + cmds = append(cmds, []string{"-C", opts.Worktree, "checkout", "-B", localBranch, "FETCH_HEAD"}) } else { - // TODO: check if non-fast-forward and suggest to use `--force` - cmds = append(cmds, []string{"merge", "--ff-only", "FETCH_HEAD"}) + fetchCmd := []string{"fetch", baseURLOrName, fmt.Sprintf("%s:%s", ref, localBranch), "--no-tags"} + if opts.Force { + fetchCmd = append(fetchCmd, "--force") + } + cmds = append(cmds, fetchCmd) + cmds = append(cmds, []string{"worktree", "add", opts.Worktree, localBranch}) } + } else if localBranch == currentBranch { + // PR head matches currently checked out branch + cmds = append(cmds, []string{"fetch", baseURLOrName, ref, "--no-tags"}) + cmds = append(cmds, syncBranchCmds("", "FETCH_HEAD", opts.Force)...) } else { // TODO: check if non-fast-forward and suggest to use `--force` fetchCmd := []string{"fetch", baseURLOrName, fmt.Sprintf("%s:%s", ref, localBranch), "--no-tags"} @@ -268,15 +326,88 @@ func localBranchExists(client *git.Client, b string) bool { return err == nil } +// isWorktreeAtPath reports whether the given path is a registered git worktree. +func isWorktreeAtPath(client *git.Client, path string) bool { + cmd, err := client.Command(context.Background(), "worktree", "list", "--porcelain") + if err != nil { + return false + } + out, err := cmd.Output() + if err != nil { + return false + } + resolved := resolvePath(path) + for _, line := range strings.Split(string(out), "\n") { + if p, ok := strings.CutPrefix(line, "worktree "); ok { + if resolvePath(p) == resolved { + return true + } + } + } + return false +} + +// syncBranchCmds returns commands that sync a branch to ref: a hard reset when +// force is set, otherwise a fast-forward-only merge. If path is non-empty, the +// commands are prefixed with -C to run inside that directory. +func syncBranchCmds(path, ref string, force bool) [][]string { + var prefix []string + if path != "" { + prefix = []string{"-C", path} + } + if force { + return [][]string{append(prefix, "reset", "--hard", ref)} + } + return [][]string{append(prefix, "merge", "--ff-only", ref)} +} + +// worktreeCheckoutCmds returns commands to switch an existing worktree to the +// given branch and sync it. Git will refuse if there are conflicting local changes. +func worktreeCheckoutCmds(path, branch, ref string, force bool) [][]string { + cmds := [][]string{{"-C", path, "checkout", branch}} + cmds = append(cmds, syncBranchCmds(path, ref, force)...) + return cmds +} + +// resolvePath canonicalizes a path for comparison against git-reported worktree +// paths. Git resolves symlinks internally, so on systems where common directories +// are symlinks (e.g. macOS /tmp -> /private/tmp), the user-provided path and the +// path git reports would otherwise not match. +func resolvePath(p string) string { + if abs, err := filepath.Abs(p); err == nil { + p = abs + } + if resolved, err := filepath.EvalSymlinks(p); err == nil { + return resolved + } + return p +} + func executeCmds(client *git.Client, credentialPattern git.CredentialPattern, cmdQueue [][]string) error { for _, args := range cmdQueue { + // Determine the git sub-command, skipping any -C prefix. + subCmd := args[0] + if len(args) >= 3 && args[0] == "-C" { + subCmd = args[2] + } + var err error var cmd *git.Command - switch args[0] { + switch subCmd { case "submodule": cmd, err = client.AuthenticatedCommand(context.Background(), credentialPattern, args...) case "fetch": - cmd, err = client.AuthenticatedCommand(context.Background(), git.AllMatchingCredentialsPattern, args...) + // AuthenticatedCommand prepends credential-helper flags + // before all args. When -C is present, strip it and + // apply as cmd.Dir so the flags don't displace it. + if args[0] == "-C" { + cmd, err = client.AuthenticatedCommand(context.Background(), git.AllMatchingCredentialsPattern, args[2:]...) + if err == nil { + cmd.Dir = args[1] + } + } else { + cmd, err = client.AuthenticatedCommand(context.Background(), git.AllMatchingCredentialsPattern, args...) + } default: cmd, err = client.Command(context.Background(), args...) } diff --git a/pkg/cmd/pr/checkout/checkout_test.go b/pkg/cmd/pr/checkout/checkout_test.go index 496139423e9..42bfe6ad155 100644 --- a/pkg/cmd/pr/checkout/checkout_test.go +++ b/pkg/cmd/pr/checkout/checkout_test.go @@ -61,6 +61,18 @@ func TestNewCmdCheckout(t *testing.T) { BranchName: "test-branch", }, }, + { + name: "worktree", + args: "--worktree /path/to/wt 123", + wantsOpts: CheckoutOptions{ + Worktree: "/path/to/wt", + }, + }, + { + name: "when --worktree is given a blank path, returns an error", + args: `--worktree "" 123`, + wantErr: cmdutil.FlagErrorf("--worktree cannot be blank"), + }, { name: "when there is no selector and no TTY, returns an error", args: "", @@ -100,6 +112,7 @@ func TestNewCmdCheckout(t *testing.T) { require.Equal(t, tt.wantsOpts.Force, spiedOpts.Force) require.Equal(t, tt.wantsOpts.Detach, spiedOpts.Detach) require.Equal(t, tt.wantsOpts.BranchName, spiedOpts.BranchName) + require.Equal(t, tt.wantsOpts.Worktree, spiedOpts.Worktree) }) } } @@ -173,6 +186,7 @@ func Test_checkoutRun(t *testing.T) { promptStubs func(*prompter.MockPrompter) remotes map[string]string + stdoutTTY bool wantStdout string wantStderr string wantErr bool @@ -293,6 +307,243 @@ func Test_checkoutRun(t *testing.T) { cs.Register(`git config branch\.foobar\.merge refs/heads/feature`, 0, "") }, }, + { + name: "checkout new branch into a worktree", + opts: &CheckoutOptions{ + Worktree: "/path/to/wt", + PRResolver: func() PRResolver { + baseRepo, pr := stubPR("OWNER/REPO:master", "OWNER/REPO:feature") + return &stubPRResolver{ + pr: pr, + baseRepo: baseRepo, + } + }(), + Config: func() (gh.Config, error) { + return config.NewBlankConfig(), nil + }, + Branch: func() (string, error) { + return "main", nil + }, + }, + remotes: map[string]string{ + "origin": "OWNER/REPO", + }, + stdoutTTY: true, + runStubs: func(cs *run.CommandStubber) { + cs.Register(`git worktree list --porcelain`, 0, "") + cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "") + cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") + cs.Register(`git worktree add --track -b feature /path/to/wt origin/feature`, 0, "") + }, + wantStdout: "✓ Worktree ready for PR #123\n /path/to/wt\n To start working: cd \"/path/to/wt\"\n", + }, + { + name: "checkout existing branch into a worktree and sync with merge", + opts: &CheckoutOptions{ + Worktree: "/path/to/wt", + PRResolver: func() PRResolver { + baseRepo, pr := stubPR("OWNER/REPO:master", "OWNER/REPO:feature") + return &stubPRResolver{ + pr: pr, + baseRepo: baseRepo, + } + }(), + Config: func() (gh.Config, error) { + return config.NewBlankConfig(), nil + }, + Branch: func() (string, error) { + return "main", nil + }, + }, + remotes: map[string]string{ + "origin": "OWNER/REPO", + }, + runStubs: func(cs *run.CommandStubber) { + cs.Register(`git worktree list --porcelain`, 0, "") + cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") + cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") + cs.Register(`git worktree add /path/to/wt feature`, 0, "") + cs.Register(`git -C /path/to/wt merge --ff-only refs/remotes/origin/feature`, 0, "") + }, + }, + { + name: "checkout existing branch into a worktree with force resets", + opts: &CheckoutOptions{ + Worktree: "/path/to/wt", + Force: true, + PRResolver: func() PRResolver { + baseRepo, pr := stubPR("OWNER/REPO:master", "OWNER/REPO:feature") + return &stubPRResolver{ + pr: pr, + baseRepo: baseRepo, + } + }(), + Config: func() (gh.Config, error) { + return config.NewBlankConfig(), nil + }, + Branch: func() (string, error) { + return "main", nil + }, + }, + remotes: map[string]string{ + "origin": "OWNER/REPO", + }, + runStubs: func(cs *run.CommandStubber) { + cs.Register(`git worktree list --porcelain`, 0, "") + cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") + cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") + cs.Register(`git worktree add /path/to/wt feature`, 0, "") + cs.Register(`git -C /path/to/wt reset --hard refs/remotes/origin/feature`, 0, "") + }, + }, + { + name: "checkout detached into a worktree", + opts: &CheckoutOptions{ + Worktree: "/path/to/wt", + Detach: true, + PRResolver: func() PRResolver { + baseRepo, pr := stubPR("OWNER/REPO:master", "OWNER/REPO:feature") + return &stubPRResolver{ + pr: pr, + baseRepo: baseRepo, + } + }(), + Config: func() (gh.Config, error) { + return config.NewBlankConfig(), nil + }, + Branch: func() (string, error) { + return "main", nil + }, + }, + remotes: map[string]string{ + "origin": "OWNER/REPO", + }, + runStubs: func(cs *run.CommandStubber) { + cs.Register(`git worktree list --porcelain`, 0, "") + cs.Register(`git fetch origin \+refs/heads/feature --no-tags`, 0, "") + cs.Register(`git worktree add --detach /path/to/wt FETCH_HEAD`, 0, "") + }, + }, + { + name: "checkout fork PR without a remote into a worktree", + opts: &CheckoutOptions{ + Worktree: "/path/to/wt", + PRResolver: func() PRResolver { + baseRepo, pr := stubPR("OWNER/REPO:master", "hubot/REPO:feature") + pr.MaintainerCanModify = true + return &stubPRResolver{ + pr: pr, + baseRepo: baseRepo, + } + }(), + Config: func() (gh.Config, error) { + return config.NewBlankConfig(), nil + }, + Branch: func() (string, error) { + return "main", nil + }, + }, + remotes: map[string]string{ + "origin": "OWNER/REPO", + }, + runStubs: func(cs *run.CommandStubber) { + cs.Register(`git worktree list --porcelain`, 0, "") + cs.Register(`git config branch\.feature\.merge`, 1, "") + cs.Register(`git fetch origin refs/pull/123/head:feature --no-tags`, 0, "") + cs.Register(`git worktree add /path/to/wt feature`, 0, "") + cs.Register(`git config branch\.feature\.remote https://github.com/hubot/REPO.git`, 0, "") + cs.Register(`git config branch\.feature\.pushRemote https://github.com/hubot/REPO.git`, 0, "") + cs.Register(`git config branch\.feature\.merge refs/heads/feature`, 0, "") + }, + }, + { + name: "checkout existing branch into the same worktree again switches and syncs it", + opts: &CheckoutOptions{ + Worktree: "/path/to/wt", + PRResolver: func() PRResolver { + baseRepo, pr := stubPR("OWNER/REPO:master", "OWNER/REPO:feature") + return &stubPRResolver{ + pr: pr, + baseRepo: baseRepo, + } + }(), + Config: func() (gh.Config, error) { + return config.NewBlankConfig(), nil + }, + Branch: func() (string, error) { + return "main", nil + }, + }, + remotes: map[string]string{ + "origin": "OWNER/REPO", + }, + runStubs: func(cs *run.CommandStubber) { + cs.Register(`git worktree list --porcelain`, 0, "worktree /path/to/wt\nHEAD deadbeef\nbranch refs/heads/feature\n") + cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") + cs.Register(`git -C /path/to/wt checkout feature`, 0, "") + cs.Register(`git -C /path/to/wt merge --ff-only refs/remotes/origin/feature`, 0, "") + }, + }, + { + name: "checkout fork PR without a remote into the same worktree again switches and syncs it", + opts: &CheckoutOptions{ + Worktree: "/path/to/wt", + PRResolver: func() PRResolver { + baseRepo, pr := stubPR("OWNER/REPO:master", "hubot/REPO:feature") + pr.MaintainerCanModify = true + return &stubPRResolver{ + pr: pr, + baseRepo: baseRepo, + } + }(), + Config: func() (gh.Config, error) { + return config.NewBlankConfig(), nil + }, + Branch: func() (string, error) { + return "main", nil + }, + }, + remotes: map[string]string{ + "origin": "OWNER/REPO", + }, + runStubs: func(cs *run.CommandStubber) { + cs.Register(`git worktree list --porcelain`, 0, "worktree /path/to/wt\nHEAD deadbeef\nbranch refs/heads/feature\n") + cs.Register(`git config branch\.feature\.merge`, 0, "refs/heads/feature") + cs.Register(`git fetch origin refs/pull/123/head --no-tags`, 0, "") + cs.Register(`git -C /path/to/wt checkout -B feature FETCH_HEAD`, 0, "") + }, + }, + { + name: "checkout with custom branch name into a worktree", + opts: &CheckoutOptions{ + Worktree: "/path/to/wt", + BranchName: "my-custom-name", + PRResolver: func() PRResolver { + baseRepo, pr := stubPR("OWNER/REPO:master", "OWNER/REPO:feature") + return &stubPRResolver{ + pr: pr, + baseRepo: baseRepo, + } + }(), + Config: func() (gh.Config, error) { + return config.NewBlankConfig(), nil + }, + Branch: func() (string, error) { + return "main", nil + }, + }, + remotes: map[string]string{ + "origin": "OWNER/REPO", + }, + stdoutTTY: true, + runStubs: func(cs *run.CommandStubber) { + cs.Register(`git worktree list --porcelain`, 0, "") + cs.Register(`git show-ref --verify -- refs/heads/my-custom-name`, 1, "") + cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") + cs.Register(`git worktree add --track -b my-custom-name /path/to/wt origin/feature`, 0, "") + }, + wantStdout: "✓ Worktree ready for PR #123\n /path/to/wt\n To start working: cd \"/path/to/wt\"\n", + }, { name: "when the PR resolver errors, then that error is bubbled up", opts: &CheckoutOptions{ @@ -309,6 +560,7 @@ func Test_checkoutRun(t *testing.T) { opts := tt.opts ios, _, stdout, stderr := iostreams.Test() + ios.SetStdoutTTY(tt.stdoutTTY) opts.IO = ios httpReg := &httpmock.Registry{} From 975faa31089d55d2f0cc4e0056a3a5432741af52 Mon Sep 17 00:00:00 2001 From: tidy-dev <75402236+tidy-dev@users.noreply.github.com> Date: Wed, 22 Jul 2026 13:02:43 -0400 Subject: [PATCH 02/25] Refine PR checkout worktree flag help --- pkg/cmd/pr/checkout/checkout.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/pr/checkout/checkout.go b/pkg/cmd/pr/checkout/checkout.go index 9dd91737afa..30016d6cfd2 100644 --- a/pkg/cmd/pr/checkout/checkout.go +++ b/pkg/cmd/pr/checkout/checkout.go @@ -103,7 +103,7 @@ func NewCmdCheckout(f *cmdutil.Factory, runF func(*CheckoutOptions) error) *cobr cmd.Flags().BoolVarP(&opts.Force, "force", "f", false, "Reset the existing local branch to the latest state of the pull request") cmd.Flags().BoolVarP(&opts.Detach, "detach", "", false, "Checkout PR with a detached HEAD") cmd.Flags().StringVarP(&opts.BranchName, "branch", "b", "", "Local branch name to use (default [the name of the head branch])") - cmd.Flags().StringVar(&opts.Worktree, "worktree", "", "Check out the pull request into a new worktree at the given `path`") + cmd.Flags().StringVar(&opts.Worktree, "worktree", "", "Check out the pull request into a worktree at the given `path`") return cmd } From 3b7f5abe7f753c5bd42fa79c1c2a02832f22d2f9 Mon Sep 17 00:00:00 2001 From: tidy-dev <75402236+tidy-dev@users.noreply.github.com> Date: Wed, 22 Jul 2026 13:20:56 -0400 Subject: [PATCH 03/25] tidying.. --- pkg/cmd/pr/checkout/checkout.go | 55 ++++++++++++++-------------- pkg/cmd/pr/checkout/checkout_test.go | 4 +- 2 files changed, 29 insertions(+), 30 deletions(-) diff --git a/pkg/cmd/pr/checkout/checkout.go b/pkg/cmd/pr/checkout/checkout.go index 30016d6cfd2..4c3f9cae244 100644 --- a/pkg/cmd/pr/checkout/checkout.go +++ b/pkg/cmd/pr/checkout/checkout.go @@ -173,9 +173,8 @@ func checkoutRun(opts *CheckoutOptions) error { if opts.Worktree != "" && opts.IO.IsStdoutTTY() { cs := opts.IO.ColorScheme() - fmt.Fprintf(opts.IO.Out, "%s Worktree ready for PR #%d\n", cs.SuccessIcon(), pr.Number) - fmt.Fprintf(opts.IO.Out, " %s\n", opts.Worktree) - fmt.Fprintf(opts.IO.Out, " To start working: cd %q\n", opts.Worktree) + fmt.Fprintf(opts.IO.ErrOut, "%s Checked out PR #%d in worktree %s\n", cs.SuccessIcon(), pr.Number, opts.Worktree) + fmt.Fprintf(opts.IO.ErrOut, " To start working: cd %s\n", opts.Worktree) } return nil @@ -198,23 +197,13 @@ func cmdsForExistingRemote(remote *cliContext.Remote, pr *api.PullRequest, opts remoteBranchRef := fmt.Sprintf("refs/remotes/%s", remoteBranch) fetchCmd := []string{"fetch", remote.Name, refSpec, "--no-tags"} - // FETCH_HEAD is per-worktree: when reusing an existing linked worktree in - // detach mode, fetch inside it so FETCH_HEAD is written there. - if opts.Detach && opts.Worktree != "" && isWorktreeAtPath(opts.GitClient, opts.Worktree) { - cmds = append(cmds, append([]string{"-C", opts.Worktree}, fetchCmd...)) - cmds = append(cmds, []string{"-C", opts.Worktree, "checkout", "--detach", "FETCH_HEAD"}) - return cmds + if opts.Detach { + return append(cmds, detachCmds(fetchCmd, opts.Worktree, opts.GitClient)...) } cmds = append(cmds, fetchCmd) switch { - case opts.Detach: - if opts.Worktree != "" { - cmds = append(cmds, []string{"worktree", "add", "--detach", opts.Worktree, "FETCH_HEAD"}) - } else { - cmds = append(cmds, []string{"checkout", "--detach", "FETCH_HEAD"}) - } case opts.Worktree != "": if isWorktreeAtPath(opts.GitClient, opts.Worktree) { cmds = append(cmds, worktreeCheckoutCmds(opts.Worktree, localBranch, remoteBranchRef, opts.Force)...) @@ -240,19 +229,7 @@ func cmdsForMissingRemote(pr *api.PullRequest, baseURLOrName, repoHost, defaultB if opts.Detach { fetchCmd := []string{"fetch", baseURLOrName, ref, "--no-tags"} - if opts.Worktree != "" && isWorktreeAtPath(opts.GitClient, opts.Worktree) { - // FETCH_HEAD is per-worktree; fetch inside the linked worktree. - cmds = append(cmds, append([]string{"-C", opts.Worktree}, fetchCmd...)) - cmds = append(cmds, []string{"-C", opts.Worktree, "checkout", "--detach", "FETCH_HEAD"}) - } else { - cmds = append(cmds, fetchCmd) - if opts.Worktree != "" { - cmds = append(cmds, []string{"worktree", "add", "--detach", opts.Worktree, "FETCH_HEAD"}) - } else { - cmds = append(cmds, []string{"checkout", "--detach", "FETCH_HEAD"}) - } - } - return cmds + return detachCmds(fetchCmd, opts.Worktree, opts.GitClient) } localBranch := pr.HeadRefName @@ -347,6 +324,28 @@ func isWorktreeAtPath(client *git.Client, path string) bool { return false } +// detachCmds returns the commands for a detached checkout. When reusing an +// existing linked worktree, FETCH_HEAD must be written inside it (it is +// per-worktree), so the fetch runs with -C . +func detachCmds(fetchCmd []string, worktree string, gitClient *git.Client) [][]string { + if worktree != "" { + if isWorktreeAtPath(gitClient, worktree) { + return [][]string{ + append([]string{"-C", worktree}, fetchCmd...), + {"-C", worktree, "checkout", "--detach", "FETCH_HEAD"}, + } + } + return [][]string{ + fetchCmd, + {"worktree", "add", "--detach", worktree, "FETCH_HEAD"}, + } + } + return [][]string{ + fetchCmd, + {"checkout", "--detach", "FETCH_HEAD"}, + } +} + // syncBranchCmds returns commands that sync a branch to ref: a hard reset when // force is set, otherwise a fast-forward-only merge. If path is non-empty, the // commands are prefixed with -C to run inside that directory. diff --git a/pkg/cmd/pr/checkout/checkout_test.go b/pkg/cmd/pr/checkout/checkout_test.go index 42bfe6ad155..45bd541a7e6 100644 --- a/pkg/cmd/pr/checkout/checkout_test.go +++ b/pkg/cmd/pr/checkout/checkout_test.go @@ -335,7 +335,7 @@ func Test_checkoutRun(t *testing.T) { cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") cs.Register(`git worktree add --track -b feature /path/to/wt origin/feature`, 0, "") }, - wantStdout: "✓ Worktree ready for PR #123\n /path/to/wt\n To start working: cd \"/path/to/wt\"\n", + wantStderr: "✓ Checked out PR #123 in worktree /path/to/wt\n To start working: cd /path/to/wt\n", }, { name: "checkout existing branch into a worktree and sync with merge", @@ -542,7 +542,7 @@ func Test_checkoutRun(t *testing.T) { cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") cs.Register(`git worktree add --track -b my-custom-name /path/to/wt origin/feature`, 0, "") }, - wantStdout: "✓ Worktree ready for PR #123\n /path/to/wt\n To start working: cd \"/path/to/wt\"\n", + wantStderr: "✓ Checked out PR #123 in worktree /path/to/wt\n To start working: cd /path/to/wt\n", }, { name: "when the PR resolver errors, then that error is bubbled up", From 9fc654ee0985d08c2d9076785c6993da885435a4 Mon Sep 17 00:00:00 2001 From: tidy-dev <75402236+tidy-dev@users.noreply.github.com> Date: Wed, 22 Jul 2026 14:19:20 -0400 Subject: [PATCH 04/25] Run submodule commands inside the worktree for pr checkout When --worktree is combined with --recurse-submodules, the submodule sync/update commands ran in the main worktree instead of the newly created one, leaving the worktree's submodules uninitialized. Prefix the submodule commands with -C (applied as cmd.Dir, mirroring the fetch handling) so they operate on the correct worktree. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/pr/checkout/checkout.go | 32 ++++++++++++++++-- pkg/cmd/pr/checkout/checkout_test.go | 49 ++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/pr/checkout/checkout.go b/pkg/cmd/pr/checkout/checkout.go index 4c3f9cae244..b88dadc0858 100644 --- a/pkg/cmd/pr/checkout/checkout.go +++ b/pkg/cmd/pr/checkout/checkout.go @@ -160,8 +160,7 @@ func checkoutRun(opts *CheckoutOptions) error { } if opts.RecurseSubmodules { - cmdQueue = append(cmdQueue, []string{"submodule", "sync", "--recursive"}) - cmdQueue = append(cmdQueue, []string{"submodule", "update", "--init", "--recursive"}) + cmdQueue = append(cmdQueue, submoduleCmds(opts.Worktree)...) } // Note that although we will probably be fetching from the head, in practice, PR checkout can only @@ -360,6 +359,23 @@ func syncBranchCmds(path, ref string, force bool) [][]string { return [][]string{append(prefix, "merge", "--ff-only", ref)} } +// submoduleCmds returns the commands to sync and update submodules. When +// worktree is non-empty, the commands are prefixed with -C so they run inside +// the worktree the PR was checked out into rather than the main worktree. +func submoduleCmds(worktree string) [][]string { + cmds := [][]string{ + {"submodule", "sync", "--recursive"}, + {"submodule", "update", "--init", "--recursive"}, + } + if worktree == "" { + return cmds + } + for i, c := range cmds { + cmds[i] = append([]string{"-C", worktree}, c...) + } + return cmds +} + // worktreeCheckoutCmds returns commands to switch an existing worktree to the // given branch and sync it. Git will refuse if there are conflicting local changes. func worktreeCheckoutCmds(path, branch, ref string, force bool) [][]string { @@ -394,7 +410,17 @@ func executeCmds(client *git.Client, credentialPattern git.CredentialPattern, cm var cmd *git.Command switch subCmd { case "submodule": - cmd, err = client.AuthenticatedCommand(context.Background(), credentialPattern, args...) + // As with fetch, strip a leading -C and apply it as + // cmd.Dir so the credential-helper flags AuthenticatedCommand + // prepends don't get displaced. + if args[0] == "-C" { + cmd, err = client.AuthenticatedCommand(context.Background(), credentialPattern, args[2:]...) + if err == nil { + cmd.Dir = args[1] + } + } else { + cmd, err = client.AuthenticatedCommand(context.Background(), credentialPattern, args...) + } case "fetch": // AuthenticatedCommand prepends credential-helper flags // before all args. When -C is present, strip it and diff --git a/pkg/cmd/pr/checkout/checkout_test.go b/pkg/cmd/pr/checkout/checkout_test.go index 45bd541a7e6..2ad7f5fc7c8 100644 --- a/pkg/cmd/pr/checkout/checkout_test.go +++ b/pkg/cmd/pr/checkout/checkout_test.go @@ -337,6 +337,39 @@ func Test_checkoutRun(t *testing.T) { }, wantStderr: "✓ Checked out PR #123 in worktree /path/to/wt\n To start working: cd /path/to/wt\n", }, + { + name: "checkout into a worktree with recurse submodules runs submodule commands inside the worktree", + opts: &CheckoutOptions{ + Worktree: "/path/to/wt", + RecurseSubmodules: true, + PRResolver: func() PRResolver { + baseRepo, pr := stubPR("OWNER/REPO:master", "OWNER/REPO:feature") + return &stubPRResolver{ + pr: pr, + baseRepo: baseRepo, + } + }(), + Config: func() (gh.Config, error) { + return config.NewBlankConfig(), nil + }, + Branch: func() (string, error) { + return "main", nil + }, + }, + remotes: map[string]string{ + "origin": "OWNER/REPO", + }, + stdoutTTY: true, + runStubs: func(cs *run.CommandStubber) { + cs.Register(`git worktree list --porcelain`, 0, "") + cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "") + cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") + cs.Register(`git worktree add --track -b feature /path/to/wt origin/feature`, 0, "") + cs.Register(`git submodule sync --recursive`, 0, "") + cs.Register(`git submodule update --init --recursive`, 0, "") + }, + wantStderr: "✓ Checked out PR #123 in worktree /path/to/wt\n To start working: cd /path/to/wt\n", + }, { name: "checkout existing branch into a worktree and sync with merge", opts: &CheckoutOptions{ @@ -1038,3 +1071,19 @@ func TestPRCheckout_detach(t *testing.T) { assert.Equal(t, "", output.String()) assert.Equal(t, "", output.Stderr()) } + +func Test_submoduleCmds(t *testing.T) { + t.Run("without worktree runs in the current directory", func(t *testing.T) { + require.Equal(t, [][]string{ + {"submodule", "sync", "--recursive"}, + {"submodule", "update", "--init", "--recursive"}, + }, submoduleCmds("")) + }) + + t.Run("with worktree prefixes -C so submodules run inside the worktree", func(t *testing.T) { + require.Equal(t, [][]string{ + {"-C", "/path/to/wt", "submodule", "sync", "--recursive"}, + {"-C", "/path/to/wt", "submodule", "update", "--init", "--recursive"}, + }, submoduleCmds("/path/to/wt")) + }) +} From 9f14d1ac675f25a75d4b940dc88e733f06398e76 Mon Sep 17 00:00:00 2001 From: tidy-dev <75402236+tidy-dev@users.noreply.github.com> Date: Wed, 22 Jul 2026 14:20:41 -0400 Subject: [PATCH 05/25] Simplify submodule worktree prefix to inline conditional Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/pr/checkout/checkout.go | 26 ++++++++------------------ pkg/cmd/pr/checkout/checkout_test.go | 16 ---------------- 2 files changed, 8 insertions(+), 34 deletions(-) diff --git a/pkg/cmd/pr/checkout/checkout.go b/pkg/cmd/pr/checkout/checkout.go index b88dadc0858..d23caed9dc1 100644 --- a/pkg/cmd/pr/checkout/checkout.go +++ b/pkg/cmd/pr/checkout/checkout.go @@ -160,7 +160,14 @@ func checkoutRun(opts *CheckoutOptions) error { } if opts.RecurseSubmodules { - cmdQueue = append(cmdQueue, submoduleCmds(opts.Worktree)...) + // Run submodule commands inside the worktree when checking out into + // one, so its submodules (not the main worktree's) get initialized. + var prefix []string + if opts.Worktree != "" { + prefix = []string{"-C", opts.Worktree} + } + cmdQueue = append(cmdQueue, append(prefix, "submodule", "sync", "--recursive")) + cmdQueue = append(cmdQueue, append(prefix, "submodule", "update", "--init", "--recursive")) } // Note that although we will probably be fetching from the head, in practice, PR checkout can only @@ -359,23 +366,6 @@ func syncBranchCmds(path, ref string, force bool) [][]string { return [][]string{append(prefix, "merge", "--ff-only", ref)} } -// submoduleCmds returns the commands to sync and update submodules. When -// worktree is non-empty, the commands are prefixed with -C so they run inside -// the worktree the PR was checked out into rather than the main worktree. -func submoduleCmds(worktree string) [][]string { - cmds := [][]string{ - {"submodule", "sync", "--recursive"}, - {"submodule", "update", "--init", "--recursive"}, - } - if worktree == "" { - return cmds - } - for i, c := range cmds { - cmds[i] = append([]string{"-C", worktree}, c...) - } - return cmds -} - // worktreeCheckoutCmds returns commands to switch an existing worktree to the // given branch and sync it. Git will refuse if there are conflicting local changes. func worktreeCheckoutCmds(path, branch, ref string, force bool) [][]string { diff --git a/pkg/cmd/pr/checkout/checkout_test.go b/pkg/cmd/pr/checkout/checkout_test.go index 2ad7f5fc7c8..06c743c0571 100644 --- a/pkg/cmd/pr/checkout/checkout_test.go +++ b/pkg/cmd/pr/checkout/checkout_test.go @@ -1071,19 +1071,3 @@ func TestPRCheckout_detach(t *testing.T) { assert.Equal(t, "", output.String()) assert.Equal(t, "", output.Stderr()) } - -func Test_submoduleCmds(t *testing.T) { - t.Run("without worktree runs in the current directory", func(t *testing.T) { - require.Equal(t, [][]string{ - {"submodule", "sync", "--recursive"}, - {"submodule", "update", "--init", "--recursive"}, - }, submoduleCmds("")) - }) - - t.Run("with worktree prefixes -C so submodules run inside the worktree", func(t *testing.T) { - require.Equal(t, [][]string{ - {"-C", "/path/to/wt", "submodule", "sync", "--recursive"}, - {"-C", "/path/to/wt", "submodule", "update", "--init", "--recursive"}, - }, submoduleCmds("/path/to/wt")) - }) -} From 927f2dd096f9de8e03234f7da93a7d360ba7c770 Mon Sep 17 00:00:00 2001 From: tidy-dev <75402236+tidy-dev@users.noreply.github.com> Date: Wed, 22 Jul 2026 14:40:08 -0400 Subject: [PATCH 06/25] Preserve no-force safety when reusing a worktree for fork PRs The missing-remote existing-worktree path used checkout -B FETCH_HEAD, which unconditionally reset the branch and could discard local commits even without --force. Switch to existence-aware logic that mirrors the non-worktree paths: when the branch exists, check it out and sync with merge --ff-only (or reset --hard under --force); only create the branch from FETCH_HEAD when it does not exist yet. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/pr/checkout/checkout.go | 15 ++++-- pkg/cmd/pr/checkout/checkout_test.go | 69 +++++++++++++++++++++++++++- 2 files changed, 79 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/pr/checkout/checkout.go b/pkg/cmd/pr/checkout/checkout.go index d23caed9dc1..949747f1dd0 100644 --- a/pkg/cmd/pr/checkout/checkout.go +++ b/pkg/cmd/pr/checkout/checkout.go @@ -254,10 +254,17 @@ func cmdsForMissingRemote(pr *api.PullRequest, baseURLOrName, repoHost, defaultB // git refuses to update a branch via refspec when it is checked // out in a worktree. cmds = append(cmds, []string{"-C", opts.Worktree, "fetch", baseURLOrName, ref, "--no-tags"}) - // Use checkout -B to create-or-reset the branch from FETCH_HEAD. - // The local branch may not exist yet (e.g. switching the worktree - // to a different fork PR). - cmds = append(cmds, []string{"-C", opts.Worktree, "checkout", "-B", localBranch, "FETCH_HEAD"}) + if localBranchExists(opts.GitClient, localBranch) { + // Branch already exists: switch to it and sync, preserving the + // no-force safety guarantee used elsewhere (ff-only merge unless + // --force, which hard-resets). + cmds = append(cmds, []string{"-C", opts.Worktree, "checkout", localBranch}) + cmds = append(cmds, syncBranchCmds(opts.Worktree, "FETCH_HEAD", opts.Force)...) + } else { + // Branch does not exist yet (e.g. switching the worktree to a + // different fork PR): create it from FETCH_HEAD. + cmds = append(cmds, []string{"-C", opts.Worktree, "checkout", "-b", localBranch, "FETCH_HEAD"}) + } } else { fetchCmd := []string{"fetch", baseURLOrName, fmt.Sprintf("%s:%s", ref, localBranch), "--no-tags"} if opts.Force { diff --git a/pkg/cmd/pr/checkout/checkout_test.go b/pkg/cmd/pr/checkout/checkout_test.go index 06c743c0571..aca51602039 100644 --- a/pkg/cmd/pr/checkout/checkout_test.go +++ b/pkg/cmd/pr/checkout/checkout_test.go @@ -541,9 +541,76 @@ func Test_checkoutRun(t *testing.T) { }, runStubs: func(cs *run.CommandStubber) { cs.Register(`git worktree list --porcelain`, 0, "worktree /path/to/wt\nHEAD deadbeef\nbranch refs/heads/feature\n") + cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") + cs.Register(`git config branch\.feature\.merge`, 0, "refs/heads/feature") + cs.Register(`git fetch origin refs/pull/123/head --no-tags`, 0, "") + cs.Register(`git -C /path/to/wt checkout feature`, 0, "") + cs.Register(`git -C /path/to/wt merge --ff-only FETCH_HEAD`, 0, "") + }, + }, + { + name: "checkout fork PR without a remote into the same worktree again with force resets", + opts: &CheckoutOptions{ + Worktree: "/path/to/wt", + Force: true, + PRResolver: func() PRResolver { + baseRepo, pr := stubPR("OWNER/REPO:master", "hubot/REPO:feature") + pr.MaintainerCanModify = true + return &stubPRResolver{ + pr: pr, + baseRepo: baseRepo, + } + }(), + Config: func() (gh.Config, error) { + return config.NewBlankConfig(), nil + }, + Branch: func() (string, error) { + return "main", nil + }, + }, + remotes: map[string]string{ + "origin": "OWNER/REPO", + }, + runStubs: func(cs *run.CommandStubber) { + cs.Register(`git worktree list --porcelain`, 0, "worktree /path/to/wt\nHEAD deadbeef\nbranch refs/heads/feature\n") + cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") cs.Register(`git config branch\.feature\.merge`, 0, "refs/heads/feature") cs.Register(`git fetch origin refs/pull/123/head --no-tags`, 0, "") - cs.Register(`git -C /path/to/wt checkout -B feature FETCH_HEAD`, 0, "") + cs.Register(`git -C /path/to/wt checkout feature`, 0, "") + cs.Register(`git -C /path/to/wt reset --hard FETCH_HEAD`, 0, "") + }, + }, + { + name: "checkout fork PR without a remote into an existing worktree whose branch does not exist yet creates it", + opts: &CheckoutOptions{ + Worktree: "/path/to/wt", + PRResolver: func() PRResolver { + baseRepo, pr := stubPR("OWNER/REPO:master", "hubot/REPO:feature") + pr.MaintainerCanModify = true + return &stubPRResolver{ + pr: pr, + baseRepo: baseRepo, + } + }(), + Config: func() (gh.Config, error) { + return config.NewBlankConfig(), nil + }, + Branch: func() (string, error) { + return "main", nil + }, + }, + remotes: map[string]string{ + "origin": "OWNER/REPO", + }, + runStubs: func(cs *run.CommandStubber) { + cs.Register(`git worktree list --porcelain`, 0, "worktree /path/to/wt\nHEAD deadbeef\nbranch refs/heads/other\n") + cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "") + cs.Register(`git config branch\.feature\.merge`, 1, "") + cs.Register(`git fetch origin refs/pull/123/head --no-tags`, 0, "") + cs.Register(`git -C /path/to/wt checkout -b feature FETCH_HEAD`, 0, "") + cs.Register(`git config branch\.feature\.remote https://github.com/hubot/REPO.git`, 0, "") + cs.Register(`git config branch\.feature\.pushRemote https://github.com/hubot/REPO.git`, 0, "") + cs.Register(`git config branch\.feature\.merge refs/heads/feature`, 0, "") }, }, { From 359fd100bbdf17836f4b482be59912f8129a6da2 Mon Sep 17 00:00:00 2001 From: tidy-dev <75402236+tidy-dev@users.noreply.github.com> Date: Wed, 22 Jul 2026 14:44:23 -0400 Subject: [PATCH 07/25] Extract authenticatedCommand helper to dedupe -C handling Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/pr/checkout/checkout.go | 40 +++++++++++++++------------------ 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/pkg/cmd/pr/checkout/checkout.go b/pkg/cmd/pr/checkout/checkout.go index 949747f1dd0..798f75e9942 100644 --- a/pkg/cmd/pr/checkout/checkout.go +++ b/pkg/cmd/pr/checkout/checkout.go @@ -407,29 +407,9 @@ func executeCmds(client *git.Client, credentialPattern git.CredentialPattern, cm var cmd *git.Command switch subCmd { case "submodule": - // As with fetch, strip a leading -C and apply it as - // cmd.Dir so the credential-helper flags AuthenticatedCommand - // prepends don't get displaced. - if args[0] == "-C" { - cmd, err = client.AuthenticatedCommand(context.Background(), credentialPattern, args[2:]...) - if err == nil { - cmd.Dir = args[1] - } - } else { - cmd, err = client.AuthenticatedCommand(context.Background(), credentialPattern, args...) - } + cmd, err = authenticatedCommand(client, credentialPattern, args) case "fetch": - // AuthenticatedCommand prepends credential-helper flags - // before all args. When -C is present, strip it and - // apply as cmd.Dir so the flags don't displace it. - if args[0] == "-C" { - cmd, err = client.AuthenticatedCommand(context.Background(), git.AllMatchingCredentialsPattern, args[2:]...) - if err == nil { - cmd.Dir = args[1] - } - } else { - cmd, err = client.AuthenticatedCommand(context.Background(), git.AllMatchingCredentialsPattern, args...) - } + cmd, err = authenticatedCommand(client, git.AllMatchingCredentialsPattern, args) default: cmd, err = client.Command(context.Background(), args...) } @@ -443,6 +423,22 @@ func executeCmds(client *git.Client, credentialPattern git.CredentialPattern, cm return nil } +// authenticatedCommand builds an authenticated git command, transparently +// handling a leading -C prefix. AuthenticatedCommand prepends +// credential-helper flags before all args, so a -C prefix would be displaced; +// instead we strip it and apply it as cmd.Dir. +func authenticatedCommand(client *git.Client, credentialPattern git.CredentialPattern, args []string) (*git.Command, error) { + if args[0] == "-C" { + cmd, err := client.AuthenticatedCommand(context.Background(), credentialPattern, args[2:]...) + if err != nil { + return nil, err + } + cmd.Dir = args[1] + return cmd, nil + } + return client.AuthenticatedCommand(context.Background(), credentialPattern, args...) +} + type PRResolver interface { Resolve() (*api.PullRequest, ghrepo.Interface, error) } From a5eea131501c535d0527eb61ade5969bd85d0ff3 Mon Sep 17 00:00:00 2001 From: tidy-dev <75402236+tidy-dev@users.noreply.github.com> Date: Wed, 22 Jul 2026 14:53:45 -0400 Subject: [PATCH 08/25] Create branch when reusing a worktree with a new --branch name The existing-remote worktree-reuse path assumed the target branch already existed and ran checkout , which failed when a new --branch name was supplied for an already-existing worktree (e.g. repointing a review worktree at a different PR). Create the branch tracking the remote when it does not exist yet, mirroring the new-worktree path. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/pr/checkout/checkout.go | 9 +++++++- pkg/cmd/pr/checkout/checkout_test.go | 32 ++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/pr/checkout/checkout.go b/pkg/cmd/pr/checkout/checkout.go index 798f75e9942..718733e4ade 100644 --- a/pkg/cmd/pr/checkout/checkout.go +++ b/pkg/cmd/pr/checkout/checkout.go @@ -212,7 +212,14 @@ func cmdsForExistingRemote(remote *cliContext.Remote, pr *api.PullRequest, opts switch { case opts.Worktree != "": if isWorktreeAtPath(opts.GitClient, opts.Worktree) { - cmds = append(cmds, worktreeCheckoutCmds(opts.Worktree, localBranch, remoteBranchRef, opts.Force)...) + if localBranchExists(opts.GitClient, localBranch) { + cmds = append(cmds, worktreeCheckoutCmds(opts.Worktree, localBranch, remoteBranchRef, opts.Force)...) + } else { + // Branch does not exist yet (e.g. reusing a worktree for a + // different PR with a new --branch name): create it tracking + // the remote branch. + cmds = append(cmds, []string{"-C", opts.Worktree, "checkout", "-b", localBranch, "--track", remoteBranch}) + } } else if localBranchExists(opts.GitClient, localBranch) { cmds = append(cmds, []string{"worktree", "add", opts.Worktree, localBranch}) cmds = append(cmds, syncBranchCmds(opts.Worktree, remoteBranchRef, opts.Force)...) diff --git a/pkg/cmd/pr/checkout/checkout_test.go b/pkg/cmd/pr/checkout/checkout_test.go index aca51602039..67e87d51ebd 100644 --- a/pkg/cmd/pr/checkout/checkout_test.go +++ b/pkg/cmd/pr/checkout/checkout_test.go @@ -512,11 +512,43 @@ func Test_checkoutRun(t *testing.T) { }, runStubs: func(cs *run.CommandStubber) { cs.Register(`git worktree list --porcelain`, 0, "worktree /path/to/wt\nHEAD deadbeef\nbranch refs/heads/feature\n") + cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") cs.Register(`git -C /path/to/wt checkout feature`, 0, "") cs.Register(`git -C /path/to/wt merge --ff-only refs/remotes/origin/feature`, 0, "") }, }, + { + name: "checkout into an existing worktree with a new custom branch name creates the branch", + opts: &CheckoutOptions{ + Worktree: "/path/to/wt", + BranchName: "my-custom-name", + PRResolver: func() PRResolver { + baseRepo, pr := stubPR("OWNER/REPO:master", "OWNER/REPO:feature") + return &stubPRResolver{ + pr: pr, + baseRepo: baseRepo, + } + }(), + Config: func() (gh.Config, error) { + return config.NewBlankConfig(), nil + }, + Branch: func() (string, error) { + return "main", nil + }, + }, + remotes: map[string]string{ + "origin": "OWNER/REPO", + }, + stdoutTTY: true, + runStubs: func(cs *run.CommandStubber) { + cs.Register(`git worktree list --porcelain`, 0, "worktree /path/to/wt\nHEAD deadbeef\nbranch refs/heads/other\n") + cs.Register(`git show-ref --verify -- refs/heads/my-custom-name`, 1, "") + cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") + cs.Register(`git -C /path/to/wt checkout -b my-custom-name --track origin/feature`, 0, "") + }, + wantStderr: "✓ Checked out PR #123 in worktree /path/to/wt\n To start working: cd /path/to/wt\n", + }, { name: "checkout fork PR without a remote into the same worktree again switches and syncs it", opts: &CheckoutOptions{ From 688751de2ce8d610ce76cd0608930e7912509ed3 Mon Sep 17 00:00:00 2001 From: tidy-dev <75402236+tidy-dev@users.noreply.github.com> Date: Wed, 22 Jul 2026 14:56:00 -0400 Subject: [PATCH 09/25] Harden worktree submodule prefixing and cover cmd.Dir stripping Use slices.Concat instead of append(prefix, ...) when building the worktree-scoped submodule commands so the shared prefix slice can never alias between the two commands. Add a unit test on authenticatedCommand asserting the leading -C is applied as cmd.Dir and stripped from the args, which the CommandStubber-based tests cannot observe. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/pr/checkout/checkout.go | 5 ++-- pkg/cmd/pr/checkout/checkout_test.go | 36 ++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/pr/checkout/checkout.go b/pkg/cmd/pr/checkout/checkout.go index 718733e4ade..3cd09428dfa 100644 --- a/pkg/cmd/pr/checkout/checkout.go +++ b/pkg/cmd/pr/checkout/checkout.go @@ -5,6 +5,7 @@ import ( "fmt" "net/http" "path/filepath" + "slices" "strings" "github.com/MakeNowJust/heredoc" @@ -166,8 +167,8 @@ func checkoutRun(opts *CheckoutOptions) error { if opts.Worktree != "" { prefix = []string{"-C", opts.Worktree} } - cmdQueue = append(cmdQueue, append(prefix, "submodule", "sync", "--recursive")) - cmdQueue = append(cmdQueue, append(prefix, "submodule", "update", "--init", "--recursive")) + cmdQueue = append(cmdQueue, slices.Concat(prefix, []string{"submodule", "sync", "--recursive"})) + cmdQueue = append(cmdQueue, slices.Concat(prefix, []string{"submodule", "update", "--init", "--recursive"})) } // Note that although we will probably be fetching from the head, in practice, PR checkout can only diff --git a/pkg/cmd/pr/checkout/checkout_test.go b/pkg/cmd/pr/checkout/checkout_test.go index 67e87d51ebd..b4ad00a3b44 100644 --- a/pkg/cmd/pr/checkout/checkout_test.go +++ b/pkg/cmd/pr/checkout/checkout_test.go @@ -1170,3 +1170,39 @@ func TestPRCheckout_detach(t *testing.T) { assert.Equal(t, "", output.String()) assert.Equal(t, "", output.Stderr()) } + +func Test_authenticatedCommand_stripsWorktreePrefix(t *testing.T) { + tests := []struct { + name string + args []string + wantDir string + wantArgs []string + }{ + { + name: "leading -C prefix is applied as cmd.Dir and stripped from args", + args: []string{"-C", "/path/to/wt", "submodule", "sync", "--recursive"}, + wantDir: "/path/to/wt", + wantArgs: []string{"submodule", "sync", "--recursive"}, + }, + { + name: "without a -C prefix cmd.Dir is left empty", + args: []string{"fetch", "origin", "refs/pull/123/head", "--no-tags"}, + wantDir: "", + wantArgs: []string{"fetch", "origin", "refs/pull/123/head", "--no-tags"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client := &git.Client{GhPath: "gh", GitPath: "git"} + cmd, err := authenticatedCommand(client, git.AllMatchingCredentialsPattern, tt.args) + require.NoError(t, err) + + assert.Equal(t, tt.wantDir, cmd.Dir) + // The credential-helper flags are prepended, so assert the tail + // carries the real sub-command args and no -C prefix leaked in. + require.GreaterOrEqual(t, len(cmd.Args), len(tt.wantArgs)) + assert.Equal(t, tt.wantArgs, cmd.Args[len(cmd.Args)-len(tt.wantArgs):]) + assert.NotContains(t, cmd.Args, "-C") + }) + } +} From 57c67f18192441ee44a39e06422c4e354a3d5234 Mon Sep 17 00:00:00 2001 From: tidy-dev <75402236+tidy-dev@users.noreply.github.com> Date: Thu, 23 Jul 2026 08:10:33 -0400 Subject: [PATCH 10/25] Cover detach-reuse, worktree fetch dir, and symlink path resolution Add the coverage gaps surfaced by review: re-running --detach against an existing worktree (the per-worktree FETCH_HEAD path), a cmd.Dir assertion for the worktree-local fetch shape (the real fork/detach production combo, not just submodule), and a symlink-resolving isWorktreeAtPath unit test so worktree reuse keeps working when git reports a canonical path but the user passes a symlinked one. Drop the custom-branch new-worktree case, which duplicated the new-branch path already covered by the existing-worktree custom-branch case. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/pr/checkout/checkout_test.go | 103 +++++++++++++++++++-------- 1 file changed, 72 insertions(+), 31 deletions(-) diff --git a/pkg/cmd/pr/checkout/checkout_test.go b/pkg/cmd/pr/checkout/checkout_test.go index b4ad00a3b44..91d9c997ec7 100644 --- a/pkg/cmd/pr/checkout/checkout_test.go +++ b/pkg/cmd/pr/checkout/checkout_test.go @@ -5,6 +5,8 @@ import ( "errors" "io" "net/http" + "os" + "path/filepath" "strings" "testing" @@ -457,6 +459,34 @@ func Test_checkoutRun(t *testing.T) { cs.Register(`git worktree add --detach /path/to/wt FETCH_HEAD`, 0, "") }, }, + { + name: "checkout detached into the same worktree again fetches and checks out inside it", + opts: &CheckoutOptions{ + Worktree: "/path/to/wt", + Detach: true, + PRResolver: func() PRResolver { + baseRepo, pr := stubPR("OWNER/REPO:master", "OWNER/REPO:feature") + return &stubPRResolver{ + pr: pr, + baseRepo: baseRepo, + } + }(), + Config: func() (gh.Config, error) { + return config.NewBlankConfig(), nil + }, + Branch: func() (string, error) { + return "main", nil + }, + }, + remotes: map[string]string{ + "origin": "OWNER/REPO", + }, + runStubs: func(cs *run.CommandStubber) { + cs.Register(`git worktree list --porcelain`, 0, "worktree /path/to/wt\nHEAD deadbeef\ndetached\n") + cs.Register(`git fetch origin \+refs/heads/feature --no-tags`, 0, "") + cs.Register(`git -C /path/to/wt checkout --detach FETCH_HEAD`, 0, "") + }, + }, { name: "checkout fork PR without a remote into a worktree", opts: &CheckoutOptions{ @@ -645,37 +675,6 @@ func Test_checkoutRun(t *testing.T) { cs.Register(`git config branch\.feature\.merge refs/heads/feature`, 0, "") }, }, - { - name: "checkout with custom branch name into a worktree", - opts: &CheckoutOptions{ - Worktree: "/path/to/wt", - BranchName: "my-custom-name", - PRResolver: func() PRResolver { - baseRepo, pr := stubPR("OWNER/REPO:master", "OWNER/REPO:feature") - return &stubPRResolver{ - pr: pr, - baseRepo: baseRepo, - } - }(), - Config: func() (gh.Config, error) { - return config.NewBlankConfig(), nil - }, - Branch: func() (string, error) { - return "main", nil - }, - }, - remotes: map[string]string{ - "origin": "OWNER/REPO", - }, - stdoutTTY: true, - runStubs: func(cs *run.CommandStubber) { - cs.Register(`git worktree list --porcelain`, 0, "") - cs.Register(`git show-ref --verify -- refs/heads/my-custom-name`, 1, "") - cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") - cs.Register(`git worktree add --track -b my-custom-name /path/to/wt origin/feature`, 0, "") - }, - wantStderr: "✓ Checked out PR #123 in worktree /path/to/wt\n To start working: cd /path/to/wt\n", - }, { name: "when the PR resolver errors, then that error is bubbled up", opts: &CheckoutOptions{ @@ -1184,6 +1183,12 @@ func Test_authenticatedCommand_stripsWorktreePrefix(t *testing.T) { wantDir: "/path/to/wt", wantArgs: []string{"submodule", "sync", "--recursive"}, }, + { + name: "leading -C prefix is applied as cmd.Dir for a worktree-local fetch", + args: []string{"-C", "/path/to/wt", "fetch", "origin", "refs/pull/123/head", "--no-tags"}, + wantDir: "/path/to/wt", + wantArgs: []string{"fetch", "origin", "refs/pull/123/head", "--no-tags"}, + }, { name: "without a -C prefix cmd.Dir is left empty", args: []string{"fetch", "origin", "refs/pull/123/head", "--no-tags"}, @@ -1206,3 +1211,39 @@ func Test_authenticatedCommand_stripsWorktreePrefix(t *testing.T) { }) } } + +func Test_isWorktreeAtPath_resolvesSymlinks(t *testing.T) { + // Git reports the canonical (symlink-resolved) worktree path, while the + // user may pass a symlinked path (e.g. macOS /tmp -> /private/tmp). The + // two must still be recognized as the same worktree. + realDir := t.TempDir() + linkDir := filepath.Join(t.TempDir(), "link") + require.NoError(t, os.Symlink(realDir, linkDir)) + + tests := []struct { + name string + input string + want bool + }{ + { + name: "symlinked input path matches git-reported canonical path", + input: linkDir, + want: true, + }, + { + name: "unrelated path does not match", + input: filepath.Join(t.TempDir(), "other"), + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cs, teardown := run.Stub() + defer teardown(t) + cs.Register(`git worktree list --porcelain`, 0, "worktree "+realDir+"\nHEAD deadbeef\nbranch refs/heads/feature\n") + + client := &git.Client{GitPath: "git"} + assert.Equal(t, tt.want, isWorktreeAtPath(client, tt.input)) + }) + } +} From 9dd2235b7aca8da51528c916f113e1b787a31ba8 Mon Sep 17 00:00:00 2001 From: tidy-dev <75402236+tidy-dev@users.noreply.github.com> Date: Wed, 29 Jul 2026 08:14:51 -0400 Subject: [PATCH 11/25] Address review: restore TODO, flatten detachCmds, guard worktree symlink - Restore the non-fast-forward // TODO breadcrumb in syncBranchCmds - Early-return the non-worktree case in detachCmds to reduce nesting - Reject a --worktree target that is a leaf symlink or non-directory via ensureWorktreePathSafe, with unit coverage Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/pr/checkout/checkout.go | 48 ++++++++++++++++++++++----- pkg/cmd/pr/checkout/checkout_test.go | 49 ++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 9 deletions(-) diff --git a/pkg/cmd/pr/checkout/checkout.go b/pkg/cmd/pr/checkout/checkout.go index 3cd09428dfa..b888b23390d 100644 --- a/pkg/cmd/pr/checkout/checkout.go +++ b/pkg/cmd/pr/checkout/checkout.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "net/http" + "os" "path/filepath" "slices" "strings" @@ -115,6 +116,12 @@ func checkoutRun(opts *CheckoutOptions) error { return err } + if opts.Worktree != "" { + if err := ensureWorktreePathSafe(opts.Worktree); err != nil { + return err + } + } + cfg, err := opts.Config() if err != nil { return err @@ -349,21 +356,22 @@ func isWorktreeAtPath(client *git.Client, path string) bool { // existing linked worktree, FETCH_HEAD must be written inside it (it is // per-worktree), so the fetch runs with -C . func detachCmds(fetchCmd []string, worktree string, gitClient *git.Client) [][]string { - if worktree != "" { - if isWorktreeAtPath(gitClient, worktree) { - return [][]string{ - append([]string{"-C", worktree}, fetchCmd...), - {"-C", worktree, "checkout", "--detach", "FETCH_HEAD"}, - } - } + if worktree == "" { return [][]string{ fetchCmd, - {"worktree", "add", "--detach", worktree, "FETCH_HEAD"}, + {"checkout", "--detach", "FETCH_HEAD"}, + } + } + + if isWorktreeAtPath(gitClient, worktree) { + return [][]string{ + append([]string{"-C", worktree}, fetchCmd...), + {"-C", worktree, "checkout", "--detach", "FETCH_HEAD"}, } } return [][]string{ fetchCmd, - {"checkout", "--detach", "FETCH_HEAD"}, + {"worktree", "add", "--detach", worktree, "FETCH_HEAD"}, } } @@ -378,6 +386,7 @@ func syncBranchCmds(path, ref string, force bool) [][]string { if force { return [][]string{append(prefix, "reset", "--hard", ref)} } + // TODO: check if non-fast-forward and suggest to use `--force` return [][]string{append(prefix, "merge", "--ff-only", ref)} } @@ -403,6 +412,27 @@ func resolvePath(p string) string { return p } +// ensureWorktreePathSafe validates a --worktree target before we write to it. +// The path must be either non-existent (git will create the worktree) or an +// existing directory, and never a symlink at its final component. A symlinked +// ancestor (e.g. macOS /tmp -> /private/tmp) is allowed; only the leaf is +// checked, using os.Lstat so a leaf symlink is not followed. Rejecting a leaf +// symlink is defense-in-depth against writing PR content through a planted link. +func ensureWorktreePathSafe(path string) error { + fi, err := os.Lstat(path) + switch { + case os.IsNotExist(err): + return nil + case err != nil: + return err + case fi.Mode()&os.ModeSymlink != 0: + return fmt.Errorf("--worktree path must not be a symlink: %s", path) + case !fi.IsDir(): + return fmt.Errorf("--worktree path must be a directory: %s", path) + } + return nil +} + func executeCmds(client *git.Client, credentialPattern git.CredentialPattern, cmdQueue [][]string) error { for _, args := range cmdQueue { // Determine the git sub-command, skipping any -C prefix. diff --git a/pkg/cmd/pr/checkout/checkout_test.go b/pkg/cmd/pr/checkout/checkout_test.go index 91d9c997ec7..8c08e1a5588 100644 --- a/pkg/cmd/pr/checkout/checkout_test.go +++ b/pkg/cmd/pr/checkout/checkout_test.go @@ -1247,3 +1247,52 @@ func Test_isWorktreeAtPath_resolvesSymlinks(t *testing.T) { }) } } + +func Test_ensureWorktreePathSafe(t *testing.T) { + base := t.TempDir() + + existingDir := filepath.Join(base, "dir") + require.NoError(t, os.Mkdir(existingDir, 0o755)) + + regularFile := filepath.Join(base, "file") + require.NoError(t, os.WriteFile(regularFile, []byte("x"), 0o644)) + + symlink := filepath.Join(base, "link") + require.NoError(t, os.Symlink(existingDir, symlink)) + + tests := []struct { + name string + path string + wantErr string + }{ + { + name: "non-existent path is allowed", + path: filepath.Join(base, "does-not-exist"), + }, + { + name: "existing directory is allowed", + path: existingDir, + }, + { + name: "leaf symlink is rejected", + path: symlink, + wantErr: "--worktree path must not be a symlink", + }, + { + name: "existing non-directory is rejected", + path: regularFile, + wantErr: "--worktree path must be a directory", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ensureWorktreePathSafe(tt.path) + if tt.wantErr == "" { + require.NoError(t, err) + return + } + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + }) + } +} From c7adccec694ff01821c63c9ccb57a6d99090925b Mon Sep 17 00:00:00 2001 From: tidy-dev <75402236+tidy-dev@users.noreply.github.com> Date: Wed, 29 Jul 2026 08:40:26 -0400 Subject: [PATCH 12/25] Detect worktrees via git rev-parse and reject the current worktree - Replace isWorktreeAtPath path-matching with git rev-parse --show-prefix --git-common-dir, letting git resolve symlinks, "..", case, and trailing slashes; delete resolvePath/EvalSymlinks - Reject a --worktree target that resolves to the current worktree, which would otherwise silently switch the current tree's branch and print a nonsensical "cd ." hint Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/pr/checkout/checkout.go | 118 ++++++++++++++++----- pkg/cmd/pr/checkout/checkout_test.go | 147 ++++++++++++++++++++++----- 2 files changed, 213 insertions(+), 52 deletions(-) diff --git a/pkg/cmd/pr/checkout/checkout.go b/pkg/cmd/pr/checkout/checkout.go index b888b23390d..c4cb03f90be 100644 --- a/pkg/cmd/pr/checkout/checkout.go +++ b/pkg/cmd/pr/checkout/checkout.go @@ -120,6 +120,9 @@ func checkoutRun(opts *CheckoutOptions) error { if err := ensureWorktreePathSafe(opts.Worktree); err != nil { return err } + if isCurrentWorktree(opts.GitClient, opts.Worktree) { + return fmt.Errorf("--worktree path is the current worktree; omit --worktree to check out here") + } } cfg, err := opts.Config() @@ -331,25 +334,106 @@ func localBranchExists(client *git.Client, b string) bool { return err == nil } -// isWorktreeAtPath reports whether the given path is a registered git worktree. +// isWorktreeAtPath reports whether path is the root of a git worktree belonging +// to this repository. Rather than enumerating and normalizing worktree paths, it +// asks git about the single directory and lets git resolve symlinks (in any path +// component), "..", case, and trailing slashes for us. func isWorktreeAtPath(client *git.Client, path string) bool { - cmd, err := client.Command(context.Background(), "worktree", "list", "--porcelain") + abs, err := filepath.Abs(path) + if err != nil { + return false + } + prefix, commonDir, err := worktreeInfoAtPath(client, abs) if err != nil { + // Non-existent and non-git directories error out here. + return false + } + // A non-empty prefix means path is a subdirectory of a worktree, not its root. + if prefix != "" { return false } + repoCommonDir, err := repoCommonDir(client) + if err != nil { + return false + } + // Confirm the worktree belongs to this repo and not an unrelated one. + return commonDir == repoCommonDir +} + +// worktreeInfoAtPath asks git about absPath and returns its prefix within the +// containing worktree (empty exactly when absPath is the worktree root) and the +// worktree's shared git common directory. Both are absolute, canonical paths. +// It returns an error for non-existent or non-git directories. +func worktreeInfoAtPath(client *git.Client, absPath string) (prefix, commonDir string, err error) { + cmd, err := client.Command(context.Background(), + "-C", absPath, + "rev-parse", "--path-format=absolute", "--show-prefix", "--git-common-dir") + if err != nil { + return "", "", err + } out, err := cmd.Output() + if err != nil { + return "", "", err + } + lines := strings.Split(strings.TrimRight(string(out), "\n"), "\n") + if len(lines) < 2 { + return "", "", fmt.Errorf("unexpected rev-parse output: %q", string(out)) + } + return lines[0], lines[len(lines)-1], nil +} + +// repoCommonDir returns the shared git common directory for the client's +// repository, as an absolute, canonical path. +func repoCommonDir(client *git.Client) (string, error) { + cmd, err := client.Command(context.Background(), + "rev-parse", "--path-format=absolute", "--git-common-dir") + if err != nil { + return "", err + } + out, err := cmd.Output() + if err != nil { + return "", err + } + return strings.TrimSpace(string(out)), nil +} + +// isCurrentWorktree reports whether path resolves to the worktree the command is +// already running in. Checking a PR out there would silently switch the current +// tree's branch, defeating the purpose of --worktree, so callers reject it. +// Detection is best-effort: if either toplevel cannot be determined (e.g. the +// path does not exist yet), it returns false so normal flow proceeds. +func isCurrentWorktree(client *git.Client, path string) bool { + abs, err := filepath.Abs(path) if err != nil { return false } - resolved := resolvePath(path) - for _, line := range strings.Split(string(out), "\n") { - if p, ok := strings.CutPrefix(line, "worktree "); ok { - if resolvePath(p) == resolved { - return true - } - } + current, err := worktreeToplevel(client, "") + if err != nil { + return false + } + target, err := worktreeToplevel(client, abs) + if err != nil { + return false + } + return current == target +} + +// worktreeToplevel returns the absolute, canonical root of the worktree +// containing dir. When dir is empty, the client's working directory is used. +func worktreeToplevel(client *git.Client, dir string) (string, error) { + args := []string{"rev-parse", "--path-format=absolute", "--show-toplevel"} + if dir != "" { + args = append([]string{"-C", dir}, args...) + } + cmd, err := client.Command(context.Background(), args...) + if err != nil { + return "", err } - return false + out, err := cmd.Output() + if err != nil { + return "", err + } + return strings.TrimSpace(string(out)), nil } // detachCmds returns the commands for a detached checkout. When reusing an @@ -398,20 +482,6 @@ func worktreeCheckoutCmds(path, branch, ref string, force bool) [][]string { return cmds } -// resolvePath canonicalizes a path for comparison against git-reported worktree -// paths. Git resolves symlinks internally, so on systems where common directories -// are symlinks (e.g. macOS /tmp -> /private/tmp), the user-provided path and the -// path git reports would otherwise not match. -func resolvePath(p string) string { - if abs, err := filepath.Abs(p); err == nil { - p = abs - } - if resolved, err := filepath.EvalSymlinks(p); err == nil { - return resolved - } - return p -} - // ensureWorktreePathSafe validates a --worktree target before we write to it. // The path must be either non-existent (git will create the worktree) or an // existing directory, and never a symlink at its final component. A symlinked diff --git a/pkg/cmd/pr/checkout/checkout_test.go b/pkg/cmd/pr/checkout/checkout_test.go index 8c08e1a5588..5dd433f7095 100644 --- a/pkg/cmd/pr/checkout/checkout_test.go +++ b/pkg/cmd/pr/checkout/checkout_test.go @@ -332,7 +332,9 @@ func Test_checkoutRun(t *testing.T) { }, stdoutTTY: true, runStubs: func(cs *run.CommandStubber) { - cs.Register(`git worktree list --porcelain`, 0, "") + cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") + cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") + cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 128, "") cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "") cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") cs.Register(`git worktree add --track -b feature /path/to/wt origin/feature`, 0, "") @@ -363,7 +365,9 @@ func Test_checkoutRun(t *testing.T) { }, stdoutTTY: true, runStubs: func(cs *run.CommandStubber) { - cs.Register(`git worktree list --porcelain`, 0, "") + cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") + cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") + cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 128, "") cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "") cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") cs.Register(`git worktree add --track -b feature /path/to/wt origin/feature`, 0, "") @@ -394,7 +398,9 @@ func Test_checkoutRun(t *testing.T) { "origin": "OWNER/REPO", }, runStubs: func(cs *run.CommandStubber) { - cs.Register(`git worktree list --porcelain`, 0, "") + cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") + cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") + cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 128, "") cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") cs.Register(`git worktree add /path/to/wt feature`, 0, "") @@ -424,7 +430,9 @@ func Test_checkoutRun(t *testing.T) { "origin": "OWNER/REPO", }, runStubs: func(cs *run.CommandStubber) { - cs.Register(`git worktree list --porcelain`, 0, "") + cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") + cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") + cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 128, "") cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") cs.Register(`git worktree add /path/to/wt feature`, 0, "") @@ -454,7 +462,9 @@ func Test_checkoutRun(t *testing.T) { "origin": "OWNER/REPO", }, runStubs: func(cs *run.CommandStubber) { - cs.Register(`git worktree list --porcelain`, 0, "") + cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") + cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") + cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 128, "") cs.Register(`git fetch origin \+refs/heads/feature --no-tags`, 0, "") cs.Register(`git worktree add --detach /path/to/wt FETCH_HEAD`, 0, "") }, @@ -482,7 +492,10 @@ func Test_checkoutRun(t *testing.T) { "origin": "OWNER/REPO", }, runStubs: func(cs *run.CommandStubber) { - cs.Register(`git worktree list --porcelain`, 0, "worktree /path/to/wt\nHEAD deadbeef\ndetached\n") + cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") + cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") + cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 0, "\n/path/to/main/.git\n") + cs.Register(`git rev-parse --path-format=absolute --git-common-dir`, 0, "/path/to/main/.git\n") cs.Register(`git fetch origin \+refs/heads/feature --no-tags`, 0, "") cs.Register(`git -C /path/to/wt checkout --detach FETCH_HEAD`, 0, "") }, @@ -510,7 +523,9 @@ func Test_checkoutRun(t *testing.T) { "origin": "OWNER/REPO", }, runStubs: func(cs *run.CommandStubber) { - cs.Register(`git worktree list --porcelain`, 0, "") + cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") + cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") + cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 128, "") cs.Register(`git config branch\.feature\.merge`, 1, "") cs.Register(`git fetch origin refs/pull/123/head:feature --no-tags`, 0, "") cs.Register(`git worktree add /path/to/wt feature`, 0, "") @@ -541,7 +556,10 @@ func Test_checkoutRun(t *testing.T) { "origin": "OWNER/REPO", }, runStubs: func(cs *run.CommandStubber) { - cs.Register(`git worktree list --porcelain`, 0, "worktree /path/to/wt\nHEAD deadbeef\nbranch refs/heads/feature\n") + cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") + cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") + cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 0, "\n/path/to/main/.git\n") + cs.Register(`git rev-parse --path-format=absolute --git-common-dir`, 0, "/path/to/main/.git\n") cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") cs.Register(`git -C /path/to/wt checkout feature`, 0, "") @@ -572,7 +590,10 @@ func Test_checkoutRun(t *testing.T) { }, stdoutTTY: true, runStubs: func(cs *run.CommandStubber) { - cs.Register(`git worktree list --porcelain`, 0, "worktree /path/to/wt\nHEAD deadbeef\nbranch refs/heads/other\n") + cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") + cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") + cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 0, "\n/path/to/main/.git\n") + cs.Register(`git rev-parse --path-format=absolute --git-common-dir`, 0, "/path/to/main/.git\n") cs.Register(`git show-ref --verify -- refs/heads/my-custom-name`, 1, "") cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") cs.Register(`git -C /path/to/wt checkout -b my-custom-name --track origin/feature`, 0, "") @@ -602,7 +623,10 @@ func Test_checkoutRun(t *testing.T) { "origin": "OWNER/REPO", }, runStubs: func(cs *run.CommandStubber) { - cs.Register(`git worktree list --porcelain`, 0, "worktree /path/to/wt\nHEAD deadbeef\nbranch refs/heads/feature\n") + cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") + cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") + cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 0, "\n/path/to/main/.git\n") + cs.Register(`git rev-parse --path-format=absolute --git-common-dir`, 0, "/path/to/main/.git\n") cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") cs.Register(`git config branch\.feature\.merge`, 0, "refs/heads/feature") cs.Register(`git fetch origin refs/pull/123/head --no-tags`, 0, "") @@ -634,7 +658,10 @@ func Test_checkoutRun(t *testing.T) { "origin": "OWNER/REPO", }, runStubs: func(cs *run.CommandStubber) { - cs.Register(`git worktree list --porcelain`, 0, "worktree /path/to/wt\nHEAD deadbeef\nbranch refs/heads/feature\n") + cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") + cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") + cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 0, "\n/path/to/main/.git\n") + cs.Register(`git rev-parse --path-format=absolute --git-common-dir`, 0, "/path/to/main/.git\n") cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") cs.Register(`git config branch\.feature\.merge`, 0, "refs/heads/feature") cs.Register(`git fetch origin refs/pull/123/head --no-tags`, 0, "") @@ -665,7 +692,10 @@ func Test_checkoutRun(t *testing.T) { "origin": "OWNER/REPO", }, runStubs: func(cs *run.CommandStubber) { - cs.Register(`git worktree list --porcelain`, 0, "worktree /path/to/wt\nHEAD deadbeef\nbranch refs/heads/other\n") + cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") + cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") + cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 0, "\n/path/to/main/.git\n") + cs.Register(`git rev-parse --path-format=absolute --git-common-dir`, 0, "/path/to/main/.git\n") cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "") cs.Register(`git config branch\.feature\.merge`, 1, "") cs.Register(`git fetch origin refs/pull/123/head --no-tags`, 0, "") @@ -1212,38 +1242,99 @@ func Test_authenticatedCommand_stripsWorktreePrefix(t *testing.T) { } } -func Test_isWorktreeAtPath_resolvesSymlinks(t *testing.T) { - // Git reports the canonical (symlink-resolved) worktree path, while the - // user may pass a symlinked path (e.g. macOS /tmp -> /private/tmp). The - // two must still be recognized as the same worktree. - realDir := t.TempDir() - linkDir := filepath.Join(t.TempDir(), "link") - require.NoError(t, os.Symlink(realDir, linkDir)) +func Test_isWorktreeAtPath(t *testing.T) { + dir := t.TempDir() + const commonDir = "/repo/.git" tests := []struct { name string - input string + stubs func(*run.CommandStubber) want bool }{ { - name: "symlinked input path matches git-reported canonical path", - input: linkDir, - want: true, + name: "worktree root of this repo", + stubs: func(cs *run.CommandStubber) { + cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 0, "\n"+commonDir+"\n") + cs.Register(`git rev-parse --path-format=absolute --git-common-dir`, 0, commonDir+"\n") + }, + want: true, }, { - name: "unrelated path does not match", - input: filepath.Join(t.TempDir(), "other"), - want: false, + name: "subdirectory of a worktree", + stubs: func(cs *run.CommandStubber) { + cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 0, "sub/\n"+commonDir+"\n") + }, + want: false, + }, + { + name: "worktree of an unrelated repo", + stubs: func(cs *run.CommandStubber) { + cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 0, "\n/other/.git\n") + cs.Register(`git rev-parse --path-format=absolute --git-common-dir`, 0, commonDir+"\n") + }, + want: false, + }, + { + name: "non-git or non-existent directory", + stubs: func(cs *run.CommandStubber) { + cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 128, "") + }, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cs, teardown := run.Stub() + defer teardown(t) + tt.stubs(cs) + + client := &git.Client{GitPath: "git"} + assert.Equal(t, tt.want, isWorktreeAtPath(client, dir)) + }) + } +} + +func Test_isCurrentWorktree(t *testing.T) { + dir := t.TempDir() + + tests := []struct { + name string + stubs func(*run.CommandStubber) + want bool + }{ + { + name: "path is the current worktree", + stubs: func(cs *run.CommandStubber) { + cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, dir+"\n") + cs.Register(`git -C .+ rev-parse --path-format=absolute --show-toplevel`, 0, dir+"\n") + }, + want: true, + }, + { + name: "path is a different worktree", + stubs: func(cs *run.CommandStubber) { + cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") + cs.Register(`git -C .+ rev-parse --path-format=absolute --show-toplevel`, 0, dir+"\n") + }, + want: false, + }, + { + name: "path is not a worktree", + stubs: func(cs *run.CommandStubber) { + cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") + cs.Register(`git -C .+ rev-parse --path-format=absolute --show-toplevel`, 128, "") + }, + want: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { cs, teardown := run.Stub() defer teardown(t) - cs.Register(`git worktree list --porcelain`, 0, "worktree "+realDir+"\nHEAD deadbeef\nbranch refs/heads/feature\n") + tt.stubs(cs) client := &git.Client{GitPath: "git"} - assert.Equal(t, tt.want, isWorktreeAtPath(client, tt.input)) + assert.Equal(t, tt.want, isCurrentWorktree(client, dir)) }) } } From c891429d14afd3f5553450907d0c7ec597ab1396 Mon Sep 17 00:00:00 2001 From: tidy-dev <75402236+tidy-dev@users.noreply.github.com> Date: Wed, 29 Jul 2026 08:50:14 -0400 Subject: [PATCH 13/25] Fix worktree toplevel stub to match Windows absolute paths isCurrentWorktree resolves the target with filepath.Abs, which yields a drive-letter path on Windows (e.g. D:\path\to\wt). Match the -C target via a wildcard so the show-toplevel stub matches on all platforms. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/pr/checkout/checkout_test.go | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/pkg/cmd/pr/checkout/checkout_test.go b/pkg/cmd/pr/checkout/checkout_test.go index 5dd433f7095..8f510e7fc4e 100644 --- a/pkg/cmd/pr/checkout/checkout_test.go +++ b/pkg/cmd/pr/checkout/checkout_test.go @@ -333,7 +333,7 @@ func Test_checkoutRun(t *testing.T) { stdoutTTY: true, runStubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") - cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") + cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 128, "") cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "") cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") @@ -366,7 +366,7 @@ func Test_checkoutRun(t *testing.T) { stdoutTTY: true, runStubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") - cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") + cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 128, "") cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "") cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") @@ -399,7 +399,7 @@ func Test_checkoutRun(t *testing.T) { }, runStubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") - cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") + cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 128, "") cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") @@ -431,7 +431,7 @@ func Test_checkoutRun(t *testing.T) { }, runStubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") - cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") + cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 128, "") cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") @@ -463,7 +463,7 @@ func Test_checkoutRun(t *testing.T) { }, runStubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") - cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") + cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 128, "") cs.Register(`git fetch origin \+refs/heads/feature --no-tags`, 0, "") cs.Register(`git worktree add --detach /path/to/wt FETCH_HEAD`, 0, "") @@ -493,7 +493,7 @@ func Test_checkoutRun(t *testing.T) { }, runStubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") - cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") + cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 0, "\n/path/to/main/.git\n") cs.Register(`git rev-parse --path-format=absolute --git-common-dir`, 0, "/path/to/main/.git\n") cs.Register(`git fetch origin \+refs/heads/feature --no-tags`, 0, "") @@ -524,7 +524,7 @@ func Test_checkoutRun(t *testing.T) { }, runStubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") - cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") + cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 128, "") cs.Register(`git config branch\.feature\.merge`, 1, "") cs.Register(`git fetch origin refs/pull/123/head:feature --no-tags`, 0, "") @@ -557,7 +557,7 @@ func Test_checkoutRun(t *testing.T) { }, runStubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") - cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") + cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 0, "\n/path/to/main/.git\n") cs.Register(`git rev-parse --path-format=absolute --git-common-dir`, 0, "/path/to/main/.git\n") cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") @@ -591,7 +591,7 @@ func Test_checkoutRun(t *testing.T) { stdoutTTY: true, runStubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") - cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") + cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 0, "\n/path/to/main/.git\n") cs.Register(`git rev-parse --path-format=absolute --git-common-dir`, 0, "/path/to/main/.git\n") cs.Register(`git show-ref --verify -- refs/heads/my-custom-name`, 1, "") @@ -624,7 +624,7 @@ func Test_checkoutRun(t *testing.T) { }, runStubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") - cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") + cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 0, "\n/path/to/main/.git\n") cs.Register(`git rev-parse --path-format=absolute --git-common-dir`, 0, "/path/to/main/.git\n") cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") @@ -659,7 +659,7 @@ func Test_checkoutRun(t *testing.T) { }, runStubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") - cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") + cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 0, "\n/path/to/main/.git\n") cs.Register(`git rev-parse --path-format=absolute --git-common-dir`, 0, "/path/to/main/.git\n") cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") @@ -693,7 +693,7 @@ func Test_checkoutRun(t *testing.T) { }, runStubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") - cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") + cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 0, "\n/path/to/main/.git\n") cs.Register(`git rev-parse --path-format=absolute --git-common-dir`, 0, "/path/to/main/.git\n") cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "") From d17503e15b471104450d87402b77f113b8920a10 Mon Sep 17 00:00:00 2001 From: tidy-dev <75402236+tidy-dev@users.noreply.github.com> Date: Wed, 29 Jul 2026 09:22:16 -0400 Subject: [PATCH 14/25] Resolve worktree target once instead of re-querying git Collapse the separate worktree-detection helpers (isWorktreeAtPath, isCurrentWorktree, worktreeToplevel, worktreeInfoAtPath, repoCommonDir) into a single resolveWorktreeTarget call made once in checkoutRun. It runs two rev-parse queries (current + target) instead of the previous four and hands the command builders a plain reuseWorktree bool, so they no longer depend on the git client for detection and stay pure. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/pr/checkout/checkout.go | 157 +++++++++++---------------- pkg/cmd/pr/checkout/checkout_test.go | 149 +++++++++---------------- 2 files changed, 117 insertions(+), 189 deletions(-) diff --git a/pkg/cmd/pr/checkout/checkout.go b/pkg/cmd/pr/checkout/checkout.go index c4cb03f90be..c40775b1514 100644 --- a/pkg/cmd/pr/checkout/checkout.go +++ b/pkg/cmd/pr/checkout/checkout.go @@ -116,13 +116,19 @@ func checkoutRun(opts *CheckoutOptions) error { return err } + var reuseWorktree bool if opts.Worktree != "" { if err := ensureWorktreePathSafe(opts.Worktree); err != nil { return err } - if isCurrentWorktree(opts.GitClient, opts.Worktree) { + target, err := resolveWorktreeTarget(opts.GitClient, opts.Worktree) + if err != nil { + return err + } + if target.isCurrent { return fmt.Errorf("--worktree path is the current worktree; omit --worktree to check out here") } + reuseWorktree = target.isRepoRoot } cfg, err := opts.Config() @@ -155,7 +161,7 @@ func checkoutRun(opts *CheckoutOptions) error { var cmdQueue [][]string if headRemote != nil { - cmdQueue = append(cmdQueue, cmdsForExistingRemote(headRemote, pr, opts)...) + cmdQueue = append(cmdQueue, cmdsForExistingRemote(headRemote, pr, opts, reuseWorktree)...) } else { httpClient, err := opts.HttpClient() if err != nil { @@ -167,7 +173,7 @@ func checkoutRun(opts *CheckoutOptions) error { if err != nil { return err } - cmdQueue = append(cmdQueue, cmdsForMissingRemote(pr, baseURLOrName, baseRepo.RepoHost(), defaultBranch, protocol, opts)...) + cmdQueue = append(cmdQueue, cmdsForMissingRemote(pr, baseURLOrName, baseRepo.RepoHost(), defaultBranch, protocol, opts, reuseWorktree)...) } if opts.RecurseSubmodules { @@ -197,7 +203,7 @@ func checkoutRun(opts *CheckoutOptions) error { return nil } -func cmdsForExistingRemote(remote *cliContext.Remote, pr *api.PullRequest, opts *CheckoutOptions) [][]string { +func cmdsForExistingRemote(remote *cliContext.Remote, pr *api.PullRequest, opts *CheckoutOptions, reuseWorktree bool) [][]string { var cmds [][]string remoteBranch := fmt.Sprintf("%s/%s", remote.Name, pr.HeadRefName) @@ -215,14 +221,14 @@ func cmdsForExistingRemote(remote *cliContext.Remote, pr *api.PullRequest, opts fetchCmd := []string{"fetch", remote.Name, refSpec, "--no-tags"} if opts.Detach { - return append(cmds, detachCmds(fetchCmd, opts.Worktree, opts.GitClient)...) + return append(cmds, detachCmds(fetchCmd, opts.Worktree, reuseWorktree)...) } cmds = append(cmds, fetchCmd) switch { case opts.Worktree != "": - if isWorktreeAtPath(opts.GitClient, opts.Worktree) { + if reuseWorktree { if localBranchExists(opts.GitClient, localBranch) { cmds = append(cmds, worktreeCheckoutCmds(opts.Worktree, localBranch, remoteBranchRef, opts.Force)...) } else { @@ -247,13 +253,13 @@ func cmdsForExistingRemote(remote *cliContext.Remote, pr *api.PullRequest, opts return cmds } -func cmdsForMissingRemote(pr *api.PullRequest, baseURLOrName, repoHost, defaultBranch, protocol string, opts *CheckoutOptions) [][]string { +func cmdsForMissingRemote(pr *api.PullRequest, baseURLOrName, repoHost, defaultBranch, protocol string, opts *CheckoutOptions, reuseWorktree bool) [][]string { var cmds [][]string ref := fmt.Sprintf("refs/pull/%d/head", pr.Number) if opts.Detach { fetchCmd := []string{"fetch", baseURLOrName, ref, "--no-tags"} - return detachCmds(fetchCmd, opts.Worktree, opts.GitClient) + return detachCmds(fetchCmd, opts.Worktree, reuseWorktree) } localBranch := pr.HeadRefName @@ -266,7 +272,7 @@ func cmdsForMissingRemote(pr *api.PullRequest, baseURLOrName, repoHost, defaultB currentBranch, _ := opts.Branch() if opts.Worktree != "" { - if isWorktreeAtPath(opts.GitClient, opts.Worktree) { + if reuseWorktree { // FETCH_HEAD is per-worktree; fetch inside the linked worktree // rather than the main worktree. We fetch to FETCH_HEAD because // git refuses to update a branch via refspec when it is checked @@ -334,112 +340,79 @@ func localBranchExists(client *git.Client, b string) bool { return err == nil } -// isWorktreeAtPath reports whether path is the root of a git worktree belonging -// to this repository. Rather than enumerating and normalizing worktree paths, it -// asks git about the single directory and lets git resolve symlinks (in any path -// component), "..", case, and trailing slashes for us. -func isWorktreeAtPath(client *git.Client, path string) bool { - abs, err := filepath.Abs(path) - if err != nil { - return false - } - prefix, commonDir, err := worktreeInfoAtPath(client, abs) - if err != nil { - // Non-existent and non-git directories error out here. - return false - } - // A non-empty prefix means path is a subdirectory of a worktree, not its root. - if prefix != "" { - return false - } - repoCommonDir, err := repoCommonDir(client) - if err != nil { - return false - } - // Confirm the worktree belongs to this repo and not an unrelated one. - return commonDir == repoCommonDir +// worktreeTarget holds what checkoutRun needs to know about a --worktree path, +// resolved once up front so the command builders stay pure and we avoid asking +// git the same questions repeatedly. +type worktreeTarget struct { + // isCurrent is true when the path resolves to the worktree the command is + // already running in. Checking a PR out there would silently switch the + // current tree's branch, defeating the purpose of --worktree, so callers + // reject it. + isCurrent bool + // isRepoRoot is true when the path is the root of an existing linked + // worktree belonging to this repository, meaning we reuse it rather than + // creating a new one. + isRepoRoot bool } -// worktreeInfoAtPath asks git about absPath and returns its prefix within the -// containing worktree (empty exactly when absPath is the worktree root) and the -// worktree's shared git common directory. Both are absolute, canonical paths. -// It returns an error for non-existent or non-git directories. -func worktreeInfoAtPath(client *git.Client, absPath string) (prefix, commonDir string, err error) { - cmd, err := client.Command(context.Background(), - "-C", absPath, - "rev-parse", "--path-format=absolute", "--show-prefix", "--git-common-dir") - if err != nil { - return "", "", err - } - out, err := cmd.Output() +// resolveWorktreeTarget asks git about path and the current worktree, letting +// git resolve symlinks (in any path component), "..", case, and trailing +// slashes for us instead of comparing paths ourselves. Detection is +// best-effort: if the current or target worktree cannot be determined (e.g. the +// path does not exist yet or is not a git directory), the corresponding flags +// stay false so normal flow proceeds and git worktree add handles the path. +func resolveWorktreeTarget(client *git.Client, path string) (worktreeTarget, error) { + var wt worktreeTarget + abs, err := filepath.Abs(path) if err != nil { - return "", "", err + return wt, err } - lines := strings.Split(strings.TrimRight(string(out), "\n"), "\n") - if len(lines) < 2 { - return "", "", fmt.Errorf("unexpected rev-parse output: %q", string(out)) - } - return lines[0], lines[len(lines)-1], nil -} -// repoCommonDir returns the shared git common directory for the client's -// repository, as an absolute, canonical path. -func repoCommonDir(client *git.Client) (string, error) { - cmd, err := client.Command(context.Background(), - "rev-parse", "--path-format=absolute", "--git-common-dir") - if err != nil { - return "", err + // Current worktree: toplevel then git-common-dir. + current, err := revParseFacts(client, "", "--show-toplevel", "--git-common-dir") + if err != nil || len(current) < 2 { + return wt, nil } - out, err := cmd.Output() - if err != nil { - return "", err - } - return strings.TrimSpace(string(out)), nil -} + currentToplevel, currentCommonDir := current[0], current[len(current)-1] -// isCurrentWorktree reports whether path resolves to the worktree the command is -// already running in. Checking a PR out there would silently switch the current -// tree's branch, defeating the purpose of --worktree, so callers reject it. -// Detection is best-effort: if either toplevel cannot be determined (e.g. the -// path does not exist yet), it returns false so normal flow proceeds. -func isCurrentWorktree(client *git.Client, path string) bool { - abs, err := filepath.Abs(path) - if err != nil { - return false - } - current, err := worktreeToplevel(client, "") - if err != nil { - return false + // Target worktree: toplevel, prefix (empty exactly at a worktree root), + // then git-common-dir. Non-existent and non-git directories error out here. + target, err := revParseFacts(client, abs, "--show-toplevel", "--show-prefix", "--git-common-dir") + if err != nil || len(target) < 3 { + return wt, nil } - target, err := worktreeToplevel(client, abs) - if err != nil { - return false - } - return current == target + targetToplevel, targetPrefix, targetCommonDir := target[0], target[1], target[2] + + wt.isCurrent = targetToplevel == currentToplevel + // A worktree root of this repo has an empty prefix and shares our common dir. + wt.isRepoRoot = targetPrefix == "" && targetCommonDir == currentCommonDir + return wt, nil } -// worktreeToplevel returns the absolute, canonical root of the worktree -// containing dir. When dir is empty, the client's working directory is used. -func worktreeToplevel(client *git.Client, dir string) (string, error) { - args := []string{"rev-parse", "--path-format=absolute", "--show-toplevel"} +// revParseFacts runs `git rev-parse --path-format=absolute ` and +// returns one output line per flag, in flag order. When dir is non-empty the +// query is scoped to that directory with -C. Results are absolute, canonical +// paths (an empty --show-prefix yields an empty line). +func revParseFacts(client *git.Client, dir string, flags ...string) ([]string, error) { + args := append([]string{"rev-parse", "--path-format=absolute"}, flags...) if dir != "" { args = append([]string{"-C", dir}, args...) } cmd, err := client.Command(context.Background(), args...) if err != nil { - return "", err + return nil, err } out, err := cmd.Output() if err != nil { - return "", err + return nil, err } - return strings.TrimSpace(string(out)), nil + return strings.Split(strings.TrimRight(string(out), "\n"), "\n"), nil } // detachCmds returns the commands for a detached checkout. When reusing an // existing linked worktree, FETCH_HEAD must be written inside it (it is // per-worktree), so the fetch runs with -C . -func detachCmds(fetchCmd []string, worktree string, gitClient *git.Client) [][]string { +func detachCmds(fetchCmd []string, worktree string, reuseWorktree bool) [][]string { if worktree == "" { return [][]string{ fetchCmd, @@ -447,7 +420,7 @@ func detachCmds(fetchCmd []string, worktree string, gitClient *git.Client) [][]s } } - if isWorktreeAtPath(gitClient, worktree) { + if reuseWorktree { return [][]string{ append([]string{"-C", worktree}, fetchCmd...), {"-C", worktree, "checkout", "--detach", "FETCH_HEAD"}, diff --git a/pkg/cmd/pr/checkout/checkout_test.go b/pkg/cmd/pr/checkout/checkout_test.go index 8f510e7fc4e..ebb16b98052 100644 --- a/pkg/cmd/pr/checkout/checkout_test.go +++ b/pkg/cmd/pr/checkout/checkout_test.go @@ -332,9 +332,8 @@ func Test_checkoutRun(t *testing.T) { }, stdoutTTY: true, runStubs: func(cs *run.CommandStubber) { - cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") - cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") - cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 128, "") + cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") + cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "") cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") cs.Register(`git worktree add --track -b feature /path/to/wt origin/feature`, 0, "") @@ -365,9 +364,8 @@ func Test_checkoutRun(t *testing.T) { }, stdoutTTY: true, runStubs: func(cs *run.CommandStubber) { - cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") - cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") - cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 128, "") + cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") + cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "") cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") cs.Register(`git worktree add --track -b feature /path/to/wt origin/feature`, 0, "") @@ -398,9 +396,8 @@ func Test_checkoutRun(t *testing.T) { "origin": "OWNER/REPO", }, runStubs: func(cs *run.CommandStubber) { - cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") - cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") - cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 128, "") + cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") + cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") cs.Register(`git worktree add /path/to/wt feature`, 0, "") @@ -430,9 +427,8 @@ func Test_checkoutRun(t *testing.T) { "origin": "OWNER/REPO", }, runStubs: func(cs *run.CommandStubber) { - cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") - cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") - cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 128, "") + cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") + cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") cs.Register(`git worktree add /path/to/wt feature`, 0, "") @@ -462,9 +458,8 @@ func Test_checkoutRun(t *testing.T) { "origin": "OWNER/REPO", }, runStubs: func(cs *run.CommandStubber) { - cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") - cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") - cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 128, "") + cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") + cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") cs.Register(`git fetch origin \+refs/heads/feature --no-tags`, 0, "") cs.Register(`git worktree add --detach /path/to/wt FETCH_HEAD`, 0, "") }, @@ -492,10 +487,8 @@ func Test_checkoutRun(t *testing.T) { "origin": "OWNER/REPO", }, runStubs: func(cs *run.CommandStubber) { - cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") - cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") - cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 0, "\n/path/to/main/.git\n") - cs.Register(`git rev-parse --path-format=absolute --git-common-dir`, 0, "/path/to/main/.git\n") + cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") + cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") cs.Register(`git fetch origin \+refs/heads/feature --no-tags`, 0, "") cs.Register(`git -C /path/to/wt checkout --detach FETCH_HEAD`, 0, "") }, @@ -523,9 +516,8 @@ func Test_checkoutRun(t *testing.T) { "origin": "OWNER/REPO", }, runStubs: func(cs *run.CommandStubber) { - cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") - cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") - cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 128, "") + cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") + cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") cs.Register(`git config branch\.feature\.merge`, 1, "") cs.Register(`git fetch origin refs/pull/123/head:feature --no-tags`, 0, "") cs.Register(`git worktree add /path/to/wt feature`, 0, "") @@ -556,10 +548,8 @@ func Test_checkoutRun(t *testing.T) { "origin": "OWNER/REPO", }, runStubs: func(cs *run.CommandStubber) { - cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") - cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") - cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 0, "\n/path/to/main/.git\n") - cs.Register(`git rev-parse --path-format=absolute --git-common-dir`, 0, "/path/to/main/.git\n") + cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") + cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") cs.Register(`git -C /path/to/wt checkout feature`, 0, "") @@ -590,10 +580,8 @@ func Test_checkoutRun(t *testing.T) { }, stdoutTTY: true, runStubs: func(cs *run.CommandStubber) { - cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") - cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") - cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 0, "\n/path/to/main/.git\n") - cs.Register(`git rev-parse --path-format=absolute --git-common-dir`, 0, "/path/to/main/.git\n") + cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") + cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") cs.Register(`git show-ref --verify -- refs/heads/my-custom-name`, 1, "") cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") cs.Register(`git -C /path/to/wt checkout -b my-custom-name --track origin/feature`, 0, "") @@ -623,10 +611,8 @@ func Test_checkoutRun(t *testing.T) { "origin": "OWNER/REPO", }, runStubs: func(cs *run.CommandStubber) { - cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") - cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") - cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 0, "\n/path/to/main/.git\n") - cs.Register(`git rev-parse --path-format=absolute --git-common-dir`, 0, "/path/to/main/.git\n") + cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") + cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") cs.Register(`git config branch\.feature\.merge`, 0, "refs/heads/feature") cs.Register(`git fetch origin refs/pull/123/head --no-tags`, 0, "") @@ -658,10 +644,8 @@ func Test_checkoutRun(t *testing.T) { "origin": "OWNER/REPO", }, runStubs: func(cs *run.CommandStubber) { - cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") - cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") - cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 0, "\n/path/to/main/.git\n") - cs.Register(`git rev-parse --path-format=absolute --git-common-dir`, 0, "/path/to/main/.git\n") + cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") + cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") cs.Register(`git config branch\.feature\.merge`, 0, "refs/heads/feature") cs.Register(`git fetch origin refs/pull/123/head --no-tags`, 0, "") @@ -692,10 +676,8 @@ func Test_checkoutRun(t *testing.T) { "origin": "OWNER/REPO", }, runStubs: func(cs *run.CommandStubber) { - cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") - cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel`, 0, "/path/to/wt\n") - cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 0, "\n/path/to/main/.git\n") - cs.Register(`git rev-parse --path-format=absolute --git-common-dir`, 0, "/path/to/main/.git\n") + cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") + cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "") cs.Register(`git config branch\.feature\.merge`, 1, "") cs.Register(`git fetch origin refs/pull/123/head --no-tags`, 0, "") @@ -1242,89 +1224,60 @@ func Test_authenticatedCommand_stripsWorktreePrefix(t *testing.T) { } } -func Test_isWorktreeAtPath(t *testing.T) { +func Test_resolveWorktreeTarget(t *testing.T) { dir := t.TempDir() - const commonDir = "/repo/.git" tests := []struct { name string stubs func(*run.CommandStubber) - want bool + want worktreeTarget }{ { - name: "worktree root of this repo", + name: "path is the current worktree", stubs: func(cs *run.CommandStubber) { - cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 0, "\n"+commonDir+"\n") - cs.Register(`git rev-parse --path-format=absolute --git-common-dir`, 0, commonDir+"\n") + cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") + cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/repo/main\n\n/repo/.git\n") }, - want: true, + want: worktreeTarget{isCurrent: true, isRepoRoot: true}, }, { - name: "subdirectory of a worktree", + name: "path is a different worktree of this repo", stubs: func(cs *run.CommandStubber) { - cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 0, "sub/\n"+commonDir+"\n") + cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") + cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") }, - want: false, + want: worktreeTarget{isCurrent: false, isRepoRoot: true}, }, { - name: "worktree of an unrelated repo", + name: "path is a subdirectory of a worktree", stubs: func(cs *run.CommandStubber) { - cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 0, "\n/other/.git\n") - cs.Register(`git rev-parse --path-format=absolute --git-common-dir`, 0, commonDir+"\n") + cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") + cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\nsub/\n/repo/.git\n") }, - want: false, + want: worktreeTarget{isCurrent: false, isRepoRoot: false}, }, { - name: "non-git or non-existent directory", + name: "path is a worktree of an unrelated repo", stubs: func(cs *run.CommandStubber) { - cs.Register(`git .+rev-parse --path-format=absolute --show-prefix --git-common-dir`, 128, "") + cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") + cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/other/wt\n\n/other/.git\n") }, - want: false, + want: worktreeTarget{isCurrent: false, isRepoRoot: false}, }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - cs, teardown := run.Stub() - defer teardown(t) - tt.stubs(cs) - - client := &git.Client{GitPath: "git"} - assert.Equal(t, tt.want, isWorktreeAtPath(client, dir)) - }) - } -} - -func Test_isCurrentWorktree(t *testing.T) { - dir := t.TempDir() - - tests := []struct { - name string - stubs func(*run.CommandStubber) - want bool - }{ { - name: "path is the current worktree", - stubs: func(cs *run.CommandStubber) { - cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, dir+"\n") - cs.Register(`git -C .+ rev-parse --path-format=absolute --show-toplevel`, 0, dir+"\n") - }, - want: true, - }, - { - name: "path is a different worktree", + name: "target is non-git or non-existent", stubs: func(cs *run.CommandStubber) { - cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") - cs.Register(`git -C .+ rev-parse --path-format=absolute --show-toplevel`, 0, dir+"\n") + cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") + cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") }, - want: false, + want: worktreeTarget{isCurrent: false, isRepoRoot: false}, }, { - name: "path is not a worktree", + name: "current worktree cannot be determined", stubs: func(cs *run.CommandStubber) { - cs.Register(`git rev-parse --path-format=absolute --show-toplevel`, 0, "/repo/main\n") - cs.Register(`git -C .+ rev-parse --path-format=absolute --show-toplevel`, 128, "") + cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 128, "") }, - want: false, + want: worktreeTarget{isCurrent: false, isRepoRoot: false}, }, } for _, tt := range tests { @@ -1334,7 +1287,9 @@ func Test_isCurrentWorktree(t *testing.T) { tt.stubs(cs) client := &git.Client{GitPath: "git"} - assert.Equal(t, tt.want, isCurrentWorktree(client, dir)) + got, err := resolveWorktreeTarget(client, dir) + require.NoError(t, err) + assert.Equal(t, tt.want, got) }) } } From 4678dcf5f72ea7ee7830bcde01f056e45e9556e4 Mon Sep 17 00:00:00 2001 From: tidy-dev <75402236+tidy-dev@users.noreply.github.com> Date: Wed, 29 Jul 2026 09:30:20 -0400 Subject: [PATCH 15/25] Trim redundant comments and clarify worktree field names Streamline comments in the worktree checkout path to only the non-obvious rationale, and rename the worktreeTarget fields to isCurrentWorktree and isExistingWorktree so they read clearly without explanation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/pr/checkout/checkout.go | 73 +++++++++++----------------- pkg/cmd/pr/checkout/checkout_test.go | 12 ++--- 2 files changed, 34 insertions(+), 51 deletions(-) diff --git a/pkg/cmd/pr/checkout/checkout.go b/pkg/cmd/pr/checkout/checkout.go index c40775b1514..41adff7ab7f 100644 --- a/pkg/cmd/pr/checkout/checkout.go +++ b/pkg/cmd/pr/checkout/checkout.go @@ -125,10 +125,10 @@ func checkoutRun(opts *CheckoutOptions) error { if err != nil { return err } - if target.isCurrent { + if target.isCurrentWorktree { return fmt.Errorf("--worktree path is the current worktree; omit --worktree to check out here") } - reuseWorktree = target.isRepoRoot + reuseWorktree = target.isExistingWorktree } cfg, err := opts.Config() @@ -232,9 +232,7 @@ func cmdsForExistingRemote(remote *cliContext.Remote, pr *api.PullRequest, opts if localBranchExists(opts.GitClient, localBranch) { cmds = append(cmds, worktreeCheckoutCmds(opts.Worktree, localBranch, remoteBranchRef, opts.Force)...) } else { - // Branch does not exist yet (e.g. reusing a worktree for a - // different PR with a new --branch name): create it tracking - // the remote branch. + // New --branch name while reusing a worktree: create it tracking the remote. cmds = append(cmds, []string{"-C", opts.Worktree, "checkout", "-b", localBranch, "--track", remoteBranch}) } } else if localBranchExists(opts.GitClient, localBranch) { @@ -273,20 +271,13 @@ func cmdsForMissingRemote(pr *api.PullRequest, baseURLOrName, repoHost, defaultB currentBranch, _ := opts.Branch() if opts.Worktree != "" { if reuseWorktree { - // FETCH_HEAD is per-worktree; fetch inside the linked worktree - // rather than the main worktree. We fetch to FETCH_HEAD because - // git refuses to update a branch via refspec when it is checked - // out in a worktree. + // FETCH_HEAD is per-worktree, and git refuses to update a branch via + // refspec while it is checked out, so fetch to FETCH_HEAD inside the worktree. cmds = append(cmds, []string{"-C", opts.Worktree, "fetch", baseURLOrName, ref, "--no-tags"}) if localBranchExists(opts.GitClient, localBranch) { - // Branch already exists: switch to it and sync, preserving the - // no-force safety guarantee used elsewhere (ff-only merge unless - // --force, which hard-resets). cmds = append(cmds, []string{"-C", opts.Worktree, "checkout", localBranch}) cmds = append(cmds, syncBranchCmds(opts.Worktree, "FETCH_HEAD", opts.Force)...) } else { - // Branch does not exist yet (e.g. switching the worktree to a - // different fork PR): create it from FETCH_HEAD. cmds = append(cmds, []string{"-C", opts.Worktree, "checkout", "-b", localBranch, "FETCH_HEAD"}) } } else { @@ -340,27 +331,22 @@ func localBranchExists(client *git.Client, b string) bool { return err == nil } -// worktreeTarget holds what checkoutRun needs to know about a --worktree path, -// resolved once up front so the command builders stay pure and we avoid asking -// git the same questions repeatedly. +// worktreeTarget describes a --worktree path, resolved once up front so the +// command builders stay pure instead of each re-querying git. type worktreeTarget struct { - // isCurrent is true when the path resolves to the worktree the command is - // already running in. Checking a PR out there would silently switch the - // current tree's branch, defeating the purpose of --worktree, so callers - // reject it. - isCurrent bool - // isRepoRoot is true when the path is the root of an existing linked - // worktree belonging to this repository, meaning we reuse it rather than - // creating a new one. - isRepoRoot bool + // isCurrentWorktree means the path is the worktree we are already running + // in. Checking out there would silently switch its branch, so callers reject it. + isCurrentWorktree bool + // isExistingWorktree means the path is the root of an existing linked + // worktree of this repo, so we reuse it rather than creating a new one. + isExistingWorktree bool } // resolveWorktreeTarget asks git about path and the current worktree, letting -// git resolve symlinks (in any path component), "..", case, and trailing -// slashes for us instead of comparing paths ourselves. Detection is -// best-effort: if the current or target worktree cannot be determined (e.g. the -// path does not exist yet or is not a git directory), the corresponding flags -// stay false so normal flow proceeds and git worktree add handles the path. +// git resolve symlinks, "..", case, and trailing slashes for us instead of +// comparing paths ourselves. Detection is best-effort: if either worktree +// cannot be determined (e.g. the path does not exist yet), the flags stay false +// so normal flow proceeds and git worktree add handles the path. func resolveWorktreeTarget(client *git.Client, path string) (worktreeTarget, error) { var wt worktreeTarget abs, err := filepath.Abs(path) @@ -368,31 +354,28 @@ func resolveWorktreeTarget(client *git.Client, path string) (worktreeTarget, err return wt, err } - // Current worktree: toplevel then git-common-dir. current, err := revParseFacts(client, "", "--show-toplevel", "--git-common-dir") if err != nil || len(current) < 2 { return wt, nil } currentToplevel, currentCommonDir := current[0], current[len(current)-1] - // Target worktree: toplevel, prefix (empty exactly at a worktree root), - // then git-common-dir. Non-existent and non-git directories error out here. + // A non-existent or non-git target errors out here, leaving both flags false. target, err := revParseFacts(client, abs, "--show-toplevel", "--show-prefix", "--git-common-dir") if err != nil || len(target) < 3 { return wt, nil } targetToplevel, targetPrefix, targetCommonDir := target[0], target[1], target[2] - wt.isCurrent = targetToplevel == currentToplevel + wt.isCurrentWorktree = targetToplevel == currentToplevel // A worktree root of this repo has an empty prefix and shares our common dir. - wt.isRepoRoot = targetPrefix == "" && targetCommonDir == currentCommonDir + wt.isExistingWorktree = targetPrefix == "" && targetCommonDir == currentCommonDir return wt, nil } // revParseFacts runs `git rev-parse --path-format=absolute ` and -// returns one output line per flag, in flag order. When dir is non-empty the -// query is scoped to that directory with -C. Results are absolute, canonical -// paths (an empty --show-prefix yields an empty line). +// returns one absolute path per flag, in flag order (an empty --show-prefix +// yields an empty string). When dir is non-empty the query is scoped there with -C. func revParseFacts(client *git.Client, dir string, flags ...string) ([]string, error) { args := append([]string{"rev-parse", "--path-format=absolute"}, flags...) if dir != "" { @@ -455,12 +438,12 @@ func worktreeCheckoutCmds(path, branch, ref string, force bool) [][]string { return cmds } -// ensureWorktreePathSafe validates a --worktree target before we write to it. -// The path must be either non-existent (git will create the worktree) or an -// existing directory, and never a symlink at its final component. A symlinked -// ancestor (e.g. macOS /tmp -> /private/tmp) is allowed; only the leaf is -// checked, using os.Lstat so a leaf symlink is not followed. Rejecting a leaf -// symlink is defense-in-depth against writing PR content through a planted link. +// ensureWorktreePathSafe validates a --worktree target before we write to it: +// it must be a non-existent path (git will create it) or an existing directory, +// and never a symlink at its final component. A symlinked ancestor (e.g. macOS +// /tmp -> /private/tmp) is fine; os.Lstat checks only the leaf so it is not +// followed. Rejecting a leaf symlink guards against writing PR content through a +// planted link. func ensureWorktreePathSafe(path string) error { fi, err := os.Lstat(path) switch { diff --git a/pkg/cmd/pr/checkout/checkout_test.go b/pkg/cmd/pr/checkout/checkout_test.go index ebb16b98052..f7fdc38f932 100644 --- a/pkg/cmd/pr/checkout/checkout_test.go +++ b/pkg/cmd/pr/checkout/checkout_test.go @@ -1238,7 +1238,7 @@ func Test_resolveWorktreeTarget(t *testing.T) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/repo/main\n\n/repo/.git\n") }, - want: worktreeTarget{isCurrent: true, isRepoRoot: true}, + want: worktreeTarget{isCurrentWorktree: true, isExistingWorktree: true}, }, { name: "path is a different worktree of this repo", @@ -1246,7 +1246,7 @@ func Test_resolveWorktreeTarget(t *testing.T) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") }, - want: worktreeTarget{isCurrent: false, isRepoRoot: true}, + want: worktreeTarget{isCurrentWorktree: false, isExistingWorktree: true}, }, { name: "path is a subdirectory of a worktree", @@ -1254,7 +1254,7 @@ func Test_resolveWorktreeTarget(t *testing.T) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\nsub/\n/repo/.git\n") }, - want: worktreeTarget{isCurrent: false, isRepoRoot: false}, + want: worktreeTarget{isCurrentWorktree: false, isExistingWorktree: false}, }, { name: "path is a worktree of an unrelated repo", @@ -1262,7 +1262,7 @@ func Test_resolveWorktreeTarget(t *testing.T) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/other/wt\n\n/other/.git\n") }, - want: worktreeTarget{isCurrent: false, isRepoRoot: false}, + want: worktreeTarget{isCurrentWorktree: false, isExistingWorktree: false}, }, { name: "target is non-git or non-existent", @@ -1270,14 +1270,14 @@ func Test_resolveWorktreeTarget(t *testing.T) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") }, - want: worktreeTarget{isCurrent: false, isRepoRoot: false}, + want: worktreeTarget{isCurrentWorktree: false, isExistingWorktree: false}, }, { name: "current worktree cannot be determined", stubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 128, "") }, - want: worktreeTarget{isCurrent: false, isRepoRoot: false}, + want: worktreeTarget{isCurrentWorktree: false, isExistingWorktree: false}, }, } for _, tt := range tests { From 95863ce00d15fabf9450ab80ee8eb1d3d7dda789 Mon Sep 17 00:00:00 2001 From: tidy-dev <75402236+tidy-dev@users.noreply.github.com> Date: Wed, 29 Jul 2026 09:33:59 -0400 Subject: [PATCH 16/25] Drop docs on self-explanatory worktree helpers Match the surrounding codebase, which rarely documents unexported helpers: remove the godoc on worktreeCheckoutCmds and tighten syncBranchCmds, keeping comments only where the rationale is non-obvious. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/pr/checkout/checkout.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/pr/checkout/checkout.go b/pkg/cmd/pr/checkout/checkout.go index 41adff7ab7f..3a5d905dbcd 100644 --- a/pkg/cmd/pr/checkout/checkout.go +++ b/pkg/cmd/pr/checkout/checkout.go @@ -415,9 +415,8 @@ func detachCmds(fetchCmd []string, worktree string, reuseWorktree bool) [][]stri } } -// syncBranchCmds returns commands that sync a branch to ref: a hard reset when -// force is set, otherwise a fast-forward-only merge. If path is non-empty, the -// commands are prefixed with -C to run inside that directory. +// syncBranchCmds syncs a branch to ref: a hard reset when force is set, +// otherwise a fast-forward-only merge. A non-empty path runs the commands there. func syncBranchCmds(path, ref string, force bool) [][]string { var prefix []string if path != "" { @@ -430,8 +429,6 @@ func syncBranchCmds(path, ref string, force bool) [][]string { return [][]string{append(prefix, "merge", "--ff-only", ref)} } -// worktreeCheckoutCmds returns commands to switch an existing worktree to the -// given branch and sync it. Git will refuse if there are conflicting local changes. func worktreeCheckoutCmds(path, branch, ref string, force bool) [][]string { cmds := [][]string{{"-C", path, "checkout", branch}} cmds = append(cmds, syncBranchCmds(path, ref, force)...) From 08b5bf2dfe7b1d1294647af2611e6ba8457d34db Mon Sep 17 00:00:00 2001 From: tidy-dev <75402236+tidy-dev@users.noreply.github.com> Date: Wed, 29 Jul 2026 09:38:37 -0400 Subject: [PATCH 17/25] Return ok bool from revParseFacts to satisfy nilerr resolveWorktreeTarget deliberately proceeds with default flags when a rev-parse fails, which the nilerr linter flagged as returning a nil error after a non-nil one. Have revParseFacts report success via an ok bool instead so the best-effort fallthrough is explicit and lint-clean. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/pr/checkout/checkout.go | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/pkg/cmd/pr/checkout/checkout.go b/pkg/cmd/pr/checkout/checkout.go index 3a5d905dbcd..422bbd011bd 100644 --- a/pkg/cmd/pr/checkout/checkout.go +++ b/pkg/cmd/pr/checkout/checkout.go @@ -354,15 +354,15 @@ func resolveWorktreeTarget(client *git.Client, path string) (worktreeTarget, err return wt, err } - current, err := revParseFacts(client, "", "--show-toplevel", "--git-common-dir") - if err != nil || len(current) < 2 { + current, ok := revParseFacts(client, "", "--show-toplevel", "--git-common-dir") + if !ok || len(current) < 2 { return wt, nil } currentToplevel, currentCommonDir := current[0], current[len(current)-1] - // A non-existent or non-git target errors out here, leaving both flags false. - target, err := revParseFacts(client, abs, "--show-toplevel", "--show-prefix", "--git-common-dir") - if err != nil || len(target) < 3 { + // A non-existent or non-git target fails here, leaving both flags false. + target, ok := revParseFacts(client, abs, "--show-toplevel", "--show-prefix", "--git-common-dir") + if !ok || len(target) < 3 { return wt, nil } targetToplevel, targetPrefix, targetCommonDir := target[0], target[1], target[2] @@ -375,21 +375,22 @@ func resolveWorktreeTarget(client *git.Client, path string) (worktreeTarget, err // revParseFacts runs `git rev-parse --path-format=absolute ` and // returns one absolute path per flag, in flag order (an empty --show-prefix -// yields an empty string). When dir is non-empty the query is scoped there with -C. -func revParseFacts(client *git.Client, dir string, flags ...string) ([]string, error) { +// yields an empty string), with ok=false if git fails. When dir is non-empty +// the query is scoped there with -C. +func revParseFacts(client *git.Client, dir string, flags ...string) (fields []string, ok bool) { args := append([]string{"rev-parse", "--path-format=absolute"}, flags...) if dir != "" { args = append([]string{"-C", dir}, args...) } cmd, err := client.Command(context.Background(), args...) if err != nil { - return nil, err + return nil, false } out, err := cmd.Output() if err != nil { - return nil, err + return nil, false } - return strings.Split(strings.TrimRight(string(out), "\n"), "\n"), nil + return strings.Split(strings.TrimRight(string(out), "\n"), "\n"), true } // detachCmds returns the commands for a detached checkout. When reusing an From f9e0ab386ffd4fae8d802902e3211764c615729e Mon Sep 17 00:00:00 2001 From: tidy-dev <75402236+tidy-dev@users.noreply.github.com> Date: Wed, 29 Jul 2026 09:39:27 -0400 Subject: [PATCH 18/25] Clarify current-worktree rejection message Reword the --worktree rejection to avoid the "current worktree" jargon, which is confusing for users who don't think of their main checkout as a worktree. Point at "the repository you're already in" instead. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/pr/checkout/checkout.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/pr/checkout/checkout.go b/pkg/cmd/pr/checkout/checkout.go index 422bbd011bd..a5ec4d39da4 100644 --- a/pkg/cmd/pr/checkout/checkout.go +++ b/pkg/cmd/pr/checkout/checkout.go @@ -126,7 +126,7 @@ func checkoutRun(opts *CheckoutOptions) error { return err } if target.isCurrentWorktree { - return fmt.Errorf("--worktree path is the current worktree; omit --worktree to check out here") + return fmt.Errorf("--worktree path points to the repository you're already in; omit --worktree to check out here") } reuseWorktree = target.isExistingWorktree } From 36a30c5aa31ccd705576500c979c8cacef7e19a1 Mon Sep 17 00:00:00 2001 From: tidy-dev <75402236+tidy-dev@users.noreply.github.com> Date: Wed, 29 Jul 2026 11:43:43 -0400 Subject: [PATCH 19/25] Bail out early on unusable --worktree paths Reject --worktree paths that point inside a different repository or nest inside an existing worktree with clear messages, instead of deferring to git (which silently creates a nested worktree or emits a generic error). Fold all rejection cases into resolveWorktreeTarget, which now returns (reuseWorktree bool, error), removing the worktreeTarget struct and simplifying the checkoutRun guard. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/pr/checkout/checkout.go | 62 +++++++++++++--------------- pkg/cmd/pr/checkout/checkout_test.go | 39 +++++++++++------ 2 files changed, 54 insertions(+), 47 deletions(-) diff --git a/pkg/cmd/pr/checkout/checkout.go b/pkg/cmd/pr/checkout/checkout.go index a5ec4d39da4..47a8ece933a 100644 --- a/pkg/cmd/pr/checkout/checkout.go +++ b/pkg/cmd/pr/checkout/checkout.go @@ -121,14 +121,10 @@ func checkoutRun(opts *CheckoutOptions) error { if err := ensureWorktreePathSafe(opts.Worktree); err != nil { return err } - target, err := resolveWorktreeTarget(opts.GitClient, opts.Worktree) + reuseWorktree, err = resolveWorktreeTarget(opts.GitClient, opts.Worktree) if err != nil { return err } - if target.isCurrentWorktree { - return fmt.Errorf("--worktree path points to the repository you're already in; omit --worktree to check out here") - } - reuseWorktree = target.isExistingWorktree } cfg, err := opts.Config() @@ -331,46 +327,44 @@ func localBranchExists(client *git.Client, b string) bool { return err == nil } -// worktreeTarget describes a --worktree path, resolved once up front so the -// command builders stay pure instead of each re-querying git. -type worktreeTarget struct { - // isCurrentWorktree means the path is the worktree we are already running - // in. Checking out there would silently switch its branch, so callers reject it. - isCurrentWorktree bool - // isExistingWorktree means the path is the root of an existing linked - // worktree of this repo, so we reuse it rather than creating a new one. - isExistingWorktree bool -} - -// resolveWorktreeTarget asks git about path and the current worktree, letting -// git resolve symlinks, "..", case, and trailing slashes for us instead of -// comparing paths ourselves. Detection is best-effort: if either worktree -// cannot be determined (e.g. the path does not exist yet), the flags stay false -// so normal flow proceeds and git worktree add handles the path. -func resolveWorktreeTarget(client *git.Client, path string) (worktreeTarget, error) { - var wt worktreeTarget +// resolveWorktreeTarget asks git where path lives, letting git resolve symlinks, +// "..", case, and trailing slashes for us instead of comparing paths ourselves. +// It returns whether an existing linked worktree there should be reused, and +// errors when the path cannot host a new worktree: a path inside a different +// repository, a subdirectory of another worktree, or the worktree we are already +// running in. Detection is best-effort: if git cannot resolve the current or +// target worktree (e.g. the path does not exist yet), reuse is false so git +// worktree add handles the path. +func resolveWorktreeTarget(client *git.Client, path string) (reuseWorktree bool, err error) { abs, err := filepath.Abs(path) if err != nil { - return wt, err + return false, err } + // git emits one line per flag, so we expect exactly two lines here. current, ok := revParseFacts(client, "", "--show-toplevel", "--git-common-dir") - if !ok || len(current) < 2 { - return wt, nil + if !ok || len(current) != 2 { + return false, nil } - currentToplevel, currentCommonDir := current[0], current[len(current)-1] + currentToplevel, currentCommonDir := current[0], current[1] - // A non-existent or non-git target fails here, leaving both flags false. + // A non-existent or non-git target fails here: it is a fresh path for a new worktree. target, ok := revParseFacts(client, abs, "--show-toplevel", "--show-prefix", "--git-common-dir") - if !ok || len(target) < 3 { - return wt, nil + if !ok || len(target) != 3 { + return false, nil } targetToplevel, targetPrefix, targetCommonDir := target[0], target[1], target[2] - wt.isCurrentWorktree = targetToplevel == currentToplevel - // A worktree root of this repo has an empty prefix and shares our common dir. - wt.isExistingWorktree = targetPrefix == "" && targetCommonDir == currentCommonDir - return wt, nil + switch { + case targetCommonDir != currentCommonDir: + return false, fmt.Errorf("--worktree path is inside a different repository") + case targetToplevel == currentToplevel: + return false, fmt.Errorf("--worktree path points to the repository you're already in; omit --worktree to check out here") + case targetPrefix != "": + return false, fmt.Errorf("--worktree path is inside an existing worktree") + } + // The path is the root of another linked worktree of this repo; reuse it. + return true, nil } // revParseFacts runs `git rev-parse --path-format=absolute ` and diff --git a/pkg/cmd/pr/checkout/checkout_test.go b/pkg/cmd/pr/checkout/checkout_test.go index f7fdc38f932..aca18753414 100644 --- a/pkg/cmd/pr/checkout/checkout_test.go +++ b/pkg/cmd/pr/checkout/checkout_test.go @@ -1228,9 +1228,10 @@ func Test_resolveWorktreeTarget(t *testing.T) { dir := t.TempDir() tests := []struct { - name string - stubs func(*run.CommandStubber) - want worktreeTarget + name string + stubs func(*run.CommandStubber) + wantReuse bool + wantErr string }{ { name: "path is the current worktree", @@ -1238,7 +1239,15 @@ func Test_resolveWorktreeTarget(t *testing.T) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/repo/main\n\n/repo/.git\n") }, - want: worktreeTarget{isCurrentWorktree: true, isExistingWorktree: true}, + wantErr: "--worktree path points to the repository you're already in; omit --worktree to check out here", + }, + { + name: "path is a subdirectory of the current worktree", + stubs: func(cs *run.CommandStubber) { + cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") + cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/repo/main\nsub/\n/repo/.git\n") + }, + wantErr: "--worktree path points to the repository you're already in; omit --worktree to check out here", }, { name: "path is a different worktree of this repo", @@ -1246,23 +1255,23 @@ func Test_resolveWorktreeTarget(t *testing.T) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") }, - want: worktreeTarget{isCurrentWorktree: false, isExistingWorktree: true}, + wantReuse: true, }, { - name: "path is a subdirectory of a worktree", + name: "path is a subdirectory of another worktree", stubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\nsub/\n/repo/.git\n") }, - want: worktreeTarget{isCurrentWorktree: false, isExistingWorktree: false}, + wantErr: "--worktree path is inside an existing worktree", }, { - name: "path is a worktree of an unrelated repo", + name: "path is inside a different repository", stubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/other/wt\n\n/other/.git\n") }, - want: worktreeTarget{isCurrentWorktree: false, isExistingWorktree: false}, + wantErr: "--worktree path is inside a different repository", }, { name: "target is non-git or non-existent", @@ -1270,14 +1279,14 @@ func Test_resolveWorktreeTarget(t *testing.T) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") }, - want: worktreeTarget{isCurrentWorktree: false, isExistingWorktree: false}, + wantReuse: false, }, { name: "current worktree cannot be determined", stubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 128, "") }, - want: worktreeTarget{isCurrentWorktree: false, isExistingWorktree: false}, + wantReuse: false, }, } for _, tt := range tests { @@ -1287,9 +1296,13 @@ func Test_resolveWorktreeTarget(t *testing.T) { tt.stubs(cs) client := &git.Client{GitPath: "git"} - got, err := resolveWorktreeTarget(client, dir) + reuse, err := resolveWorktreeTarget(client, dir) + if tt.wantErr != "" { + require.EqualError(t, err, tt.wantErr) + return + } require.NoError(t, err) - assert.Equal(t, tt.want, got) + assert.Equal(t, tt.wantReuse, reuse) }) } } From 83ebb8c679ef284fda30e56ad42358015500f1f1 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Mon, 3 Aug 2026 15:57:42 +0100 Subject: [PATCH 20/25] test(pr/checkout): add acceptance tests for worktree checkout Cover checking out a PR into a new git worktree, reusing an existing worktree, detached checkouts, checkouts of fork PRs whose head repo is not a configured remote, and force syncing across diverging branches. Each scenario also asserts the main working copy's branch is untouched. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6e4cb568-27f3-4c7e-a344-859e2c2d69b0 --- .../pr/pr-checkout-worktree-detach.txtar | 56 ++++++++ .../pr/pr-checkout-worktree-from-fork.txtar | 55 +++++++ .../testdata/pr/pr-checkout-worktree.txtar | 135 ++++++++++++++++++ 3 files changed, 246 insertions(+) create mode 100644 acceptance/testdata/pr/pr-checkout-worktree-detach.txtar create mode 100644 acceptance/testdata/pr/pr-checkout-worktree-from-fork.txtar create mode 100644 acceptance/testdata/pr/pr-checkout-worktree.txtar diff --git a/acceptance/testdata/pr/pr-checkout-worktree-detach.txtar b/acceptance/testdata/pr/pr-checkout-worktree-detach.txtar new file mode 100644 index 00000000000..c0feec4e2e4 --- /dev/null +++ b/acceptance/testdata/pr/pr-checkout-worktree-detach.txtar @@ -0,0 +1,56 @@ +# Checkout a PR into a worktree with a detached HEAD, then reuse that worktree. + +# Set up env vars +env REPO=${SCRIPT_NAME}-${RANDOM_STRING} + +# Use gh as a credential helper +exec gh auth setup-git + +# Create a repository with a file so it has a default branch +exec gh repo create ${ORG}/${REPO} --add-readme --private + +# Defer repo cleanup +defer gh repo delete --yes ${ORG}/${REPO} + +# Clone the repo +exec gh repo clone ${ORG}/${REPO} + +# Prepare a branch to PR +cd ${REPO} +exec git checkout -b feature-branch +exec git commit --allow-empty -m 'Empty Commit' +exec git push -u origin feature-branch + +# Create the PR +exec gh pr create --title 'Feature Title' --body 'Feature Body' +stdout2env PR_URL + +# Return to the default branch +exec git checkout main + +# Checkout the PR into a fresh worktree with a detached HEAD +exec gh pr checkout ${PR_URL} --detach --worktree ../wt +exists ../wt + +# The worktree HEAD is detached, so it has no symbolic ref +! exec git -C ../wt symbolic-ref -q HEAD + +# The detached HEAD points at the PR head commit +exec git -C ../wt rev-parse HEAD +stdout2env WT_HEAD +exec git rev-parse origin/feature-branch +stdout ${WT_HEAD} + +# The main working copy is left untouched +exec git rev-parse --abbrev-ref HEAD +stdout '(?m)^main$' + +# Checking out again into the same worktree reuses it and stays detached +exec gh pr checkout ${PR_URL} --detach --worktree ../wt +! exec git -C ../wt symbolic-ref -q HEAD +exec git -C ../wt rev-parse HEAD +stdout ${WT_HEAD} + +# The main working copy is left untouched +exec git rev-parse --abbrev-ref HEAD +stdout '(?m)^main$' diff --git a/acceptance/testdata/pr/pr-checkout-worktree-from-fork.txtar b/acceptance/testdata/pr/pr-checkout-worktree-from-fork.txtar new file mode 100644 index 00000000000..8d9fc89cc2e --- /dev/null +++ b/acceptance/testdata/pr/pr-checkout-worktree-from-fork.txtar @@ -0,0 +1,55 @@ +# Checkout a fork PR whose head repository is not configured as a remote into a +# worktree, then reuse that worktree. + +# Set up env vars +env REPO=${SCRIPT_NAME}-${RANDOM_STRING} + +# Use gh as a credential helper +exec gh auth setup-git + +# Create a repository with a file so it has a default branch +exec gh repo create ${ORG}/${REPO} --add-readme --private + +# Defer upstream cleanup +defer gh repo delete --yes ${ORG}/${REPO} + +# Create a fork +exec gh repo fork ${ORG}/${REPO} --org ${ORG} --fork-name ${REPO}-fork +sleep 5 + +# Defer fork cleanup +defer gh repo delete --yes ${ORG}/${REPO}-fork + +# Clone both repos +exec gh repo clone ${ORG}/${REPO} +exec gh repo clone ${ORG}/${REPO}-fork + +# Prepare a branch to PR in the fork itself +cd ${REPO}-fork +exec git checkout -b feature-branch +exec git commit --allow-empty -m 'Empty Commit' +exec git push -u origin feature-branch + +exec gh repo set-default ${ORG}/${REPO}-fork +exec gh pr create --title 'Feature Title' --body 'Feature Body' +stdout2env PR_URL + +# From the upstream clone, where the fork is not a remote, check out the PR into a worktree +cd ${WORK}/${REPO} +exec gh pr checkout ${PR_URL} --worktree ../wt +exists ../wt +exec git -C ../wt rev-parse --abbrev-ref HEAD +stdout '(?m)^feature-branch$' + +# The main working copy is left untouched +exec git rev-parse --abbrev-ref HEAD +stdout '(?m)^main$' + +# Reusing the same worktree for the same fork PR works +exec gh pr checkout ${PR_URL} --worktree ../wt +exec git -C ../wt rev-parse --abbrev-ref HEAD +stdout '(?m)^feature-branch$' + +# The main working copy is left untouched +exec git rev-parse --abbrev-ref HEAD +stdout '(?m)^main$' diff --git a/acceptance/testdata/pr/pr-checkout-worktree.txtar b/acceptance/testdata/pr/pr-checkout-worktree.txtar new file mode 100644 index 00000000000..f16a2dd3373 --- /dev/null +++ b/acceptance/testdata/pr/pr-checkout-worktree.txtar @@ -0,0 +1,135 @@ +# Checkout a PR into a git worktree, then reuse that worktree, rename its branch, +# force sync it, and add a second worktree once the local branch already exists. + +# Set up env vars +env REPO=${SCRIPT_NAME}-${RANDOM_STRING} + +# Use gh as a credential helper +exec gh auth setup-git + +# Create a repository with a file so it has a default branch +exec gh repo create ${ORG}/${REPO} --add-readme --private + +# Defer repo cleanup +defer gh repo delete --yes ${ORG}/${REPO} + +# Clone the repo +exec gh repo clone ${ORG}/${REPO} + +# Prepare a branch to PR +cd ${REPO} +exec git checkout -b feature-branch +exec git commit --allow-empty -m 'Empty Commit' +exec git push -u origin feature-branch + +# Create the PR +exec gh pr create --title 'Feature Title' --body 'Feature Body' +stdout2env PR_URL + +# Remove the local branch so checkout has to create it from the remote +exec git checkout main +exec git branch -D feature-branch +stdout 'Deleted branch feature-branch' + +# Checkout the PR into a fresh worktree +exec gh pr checkout ${PR_URL} --worktree ../wt +exists ../wt + +# The worktree is on the PR branch +exec git -C ../wt rev-parse --abbrev-ref HEAD +stdout '(?m)^feature-branch$' + +# The main working copy stays on the default branch +exec git rev-parse --abbrev-ref HEAD +stdout '(?m)^main$' + +# Checking out the same PR into the same worktree again reuses it +exec gh pr checkout ${PR_URL} --worktree ../wt +exec git -C ../wt rev-parse --abbrev-ref HEAD +stdout '(?m)^feature-branch$' + +# The main working copy is left untouched +exec git rev-parse --abbrev-ref HEAD +stdout '(?m)^main$' + +# Checking out into the reused worktree with a new branch name creates that branch +exec gh pr checkout ${PR_URL} --worktree ../wt --branch renamed-branch +exec git -C ../wt rev-parse --abbrev-ref HEAD +stdout '(?m)^renamed-branch$' + +# The main working copy is left untouched +exec git rev-parse --abbrev-ref HEAD +stdout '(?m)^main$' + +# Give the worktree's PR branch a local commit so it diverges from the PR head. +# A plain reuse would fast-forward-only merge and keep this commit, so --force is +# required to discard it with a hard reset. +exec git -C ../wt checkout feature-branch +exec git -C ../wt commit --allow-empty -m 'Diverging local commit' + +# Force checking out the PR into the reused worktree hard resets it to the PR head +exec gh pr checkout ${PR_URL} --worktree ../wt --force +exec git -C ../wt rev-parse --abbrev-ref HEAD +stdout '(?m)^feature-branch$' + +# The diverging local commit was discarded by the hard reset +exec git -C ../wt log -1 --format=%s +! stdout 'Diverging local commit' + +# The worktree branch now matches the PR head +exec git -C ../wt rev-parse HEAD +stdout2env WT_HEAD +exec git rev-parse origin/feature-branch +stdout ${WT_HEAD} + +# The main working copy is left untouched +exec git rev-parse --abbrev-ref HEAD +stdout '(?m)^main$' + +# With the local branch now present, removing the worktree and checking out into a +# fresh path adds a new worktree for the existing branch +exec git worktree remove ../wt +exec gh pr checkout ${PR_URL} --worktree ../wt2 +exists ../wt2 +exec git -C ../wt2 rev-parse --abbrev-ref HEAD +stdout '(?m)^feature-branch$' + +# The main working copy is left untouched +exec git rev-parse --abbrev-ref HEAD +stdout '(?m)^main$' + +# Now create a two-sided divergence: advance the remote PR branch with a pushed +# commit the local branch will not have... +exec git -C ../wt2 commit --allow-empty -m 'Remote commit' +exec git -C ../wt2 push origin feature-branch + +# ...then rewind the local branch and give it a different, local-only commit, so +# neither branch is an ancestor of the other +exec git -C ../wt2 reset --hard HEAD~1 +exec git -C ../wt2 commit --allow-empty -m 'Local commit' + +# A non-force checkout cannot fast-forward across the divergence and fails +! exec gh pr checkout ${PR_URL} --worktree ../wt2 +stderr 'Not possible to fast-forward' + +# The local-only commit is still there because the failed sync changed nothing +exec git -C ../wt2 log -1 --format=%s +stdout 'Local commit' + +# Forcing the checkout hard resets the branch to the advanced PR head +exec gh pr checkout ${PR_URL} --worktree ../wt2 --force +exec git -C ../wt2 rev-parse --abbrev-ref HEAD +stdout '(?m)^feature-branch$' +exec git -C ../wt2 log -1 --format=%s +stdout 'Remote commit' +! stdout 'Local commit' + +# The worktree branch now matches the advanced PR head +exec git -C ../wt2 rev-parse HEAD +stdout2env WT2_HEAD +exec git rev-parse origin/feature-branch +stdout ${WT2_HEAD} + +# The main working copy is left untouched +exec git rev-parse --abbrev-ref HEAD +stdout '(?m)^main$' From d2f477ec706553f5d8565da6dadbb945631c139f Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Tue, 4 Aug 2026 10:46:24 +0100 Subject: [PATCH 21/25] chore(pr checkout): polish worktree related tests Signed-off-by: Babak K. Shandiz --- pkg/cmd/pr/checkout/checkout_test.go | 44 ++++++++++++++++------------ 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/pkg/cmd/pr/checkout/checkout_test.go b/pkg/cmd/pr/checkout/checkout_test.go index aca18753414..55369606abc 100644 --- a/pkg/cmd/pr/checkout/checkout_test.go +++ b/pkg/cmd/pr/checkout/checkout_test.go @@ -333,7 +333,7 @@ func Test_checkoutRun(t *testing.T) { stdoutTTY: true, runStubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") - cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") + cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "") cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") cs.Register(`git worktree add --track -b feature /path/to/wt origin/feature`, 0, "") @@ -365,7 +365,7 @@ func Test_checkoutRun(t *testing.T) { stdoutTTY: true, runStubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") - cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") + cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "") cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") cs.Register(`git worktree add --track -b feature /path/to/wt origin/feature`, 0, "") @@ -397,7 +397,7 @@ func Test_checkoutRun(t *testing.T) { }, runStubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") - cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") + cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") cs.Register(`git worktree add /path/to/wt feature`, 0, "") @@ -428,7 +428,7 @@ func Test_checkoutRun(t *testing.T) { }, runStubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") - cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") + cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") cs.Register(`git worktree add /path/to/wt feature`, 0, "") @@ -459,7 +459,7 @@ func Test_checkoutRun(t *testing.T) { }, runStubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") - cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") + cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") cs.Register(`git fetch origin \+refs/heads/feature --no-tags`, 0, "") cs.Register(`git worktree add --detach /path/to/wt FETCH_HEAD`, 0, "") }, @@ -488,7 +488,7 @@ func Test_checkoutRun(t *testing.T) { }, runStubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") - cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") + cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") cs.Register(`git fetch origin \+refs/heads/feature --no-tags`, 0, "") cs.Register(`git -C /path/to/wt checkout --detach FETCH_HEAD`, 0, "") }, @@ -517,7 +517,7 @@ func Test_checkoutRun(t *testing.T) { }, runStubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") - cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") + cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") cs.Register(`git config branch\.feature\.merge`, 1, "") cs.Register(`git fetch origin refs/pull/123/head:feature --no-tags`, 0, "") cs.Register(`git worktree add /path/to/wt feature`, 0, "") @@ -549,7 +549,7 @@ func Test_checkoutRun(t *testing.T) { }, runStubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") - cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") + cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") cs.Register(`git -C /path/to/wt checkout feature`, 0, "") @@ -581,7 +581,7 @@ func Test_checkoutRun(t *testing.T) { stdoutTTY: true, runStubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") - cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") + cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") cs.Register(`git show-ref --verify -- refs/heads/my-custom-name`, 1, "") cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") cs.Register(`git -C /path/to/wt checkout -b my-custom-name --track origin/feature`, 0, "") @@ -612,7 +612,7 @@ func Test_checkoutRun(t *testing.T) { }, runStubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") - cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") + cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") cs.Register(`git config branch\.feature\.merge`, 0, "refs/heads/feature") cs.Register(`git fetch origin refs/pull/123/head --no-tags`, 0, "") @@ -645,7 +645,7 @@ func Test_checkoutRun(t *testing.T) { }, runStubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") - cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") + cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") cs.Register(`git config branch\.feature\.merge`, 0, "refs/heads/feature") cs.Register(`git fetch origin refs/pull/123/head --no-tags`, 0, "") @@ -677,7 +677,7 @@ func Test_checkoutRun(t *testing.T) { }, runStubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") - cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") + cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "") cs.Register(`git config branch\.feature\.merge`, 1, "") cs.Register(`git fetch origin refs/pull/123/head --no-tags`, 0, "") @@ -1210,7 +1210,10 @@ func Test_authenticatedCommand_stripsWorktreePrefix(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - client := &git.Client{GhPath: "gh", GitPath: "git"} + client := &git.Client{ + GhPath: "/some/path/gh", + GitPath: "/some/path/git", + } cmd, err := authenticatedCommand(client, git.AllMatchingCredentialsPattern, tt.args) require.NoError(t, err) @@ -1245,7 +1248,7 @@ func Test_resolveWorktreeTarget(t *testing.T) { name: "path is a subdirectory of the current worktree", stubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") - cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/repo/main\nsub/\n/repo/.git\n") + cs.Register(`git -C .+ rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/repo/main\nsub/\n/repo/.git\n") }, wantErr: "--worktree path points to the repository you're already in; omit --worktree to check out here", }, @@ -1253,7 +1256,7 @@ func Test_resolveWorktreeTarget(t *testing.T) { name: "path is a different worktree of this repo", stubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") - cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") + cs.Register(`git -C .+ rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") }, wantReuse: true, }, @@ -1261,7 +1264,7 @@ func Test_resolveWorktreeTarget(t *testing.T) { name: "path is a subdirectory of another worktree", stubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") - cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\nsub/\n/repo/.git\n") + cs.Register(`git -C .+ rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\nsub/\n/repo/.git\n") }, wantErr: "--worktree path is inside an existing worktree", }, @@ -1269,7 +1272,7 @@ func Test_resolveWorktreeTarget(t *testing.T) { name: "path is inside a different repository", stubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") - cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/other/wt\n\n/other/.git\n") + cs.Register(`git -C .+ rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/other/wt\n\n/other/.git\n") }, wantErr: "--worktree path is inside a different repository", }, @@ -1277,7 +1280,7 @@ func Test_resolveWorktreeTarget(t *testing.T) { name: "target is non-git or non-existent", stubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") - cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") + cs.Register(`git -C .+ rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") }, wantReuse: false, }, @@ -1295,7 +1298,10 @@ func Test_resolveWorktreeTarget(t *testing.T) { defer teardown(t) tt.stubs(cs) - client := &git.Client{GitPath: "git"} + client := &git.Client{ + GhPath: "/some/path/gh", + GitPath: "/some/path/git", + } reuse, err := resolveWorktreeTarget(client, dir) if tt.wantErr != "" { require.EqualError(t, err, tt.wantErr) From 8181d215974efca1044cacb97f8ae6c14288e4aa Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Tue, 4 Aug 2026 11:14:18 +0100 Subject: [PATCH 22/25] docs(skills): mention pr checkout worktree support Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6e4cb568-27f3-4c7e-a344-859e2c2d69b0 --- skills/gh/SKILL.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/skills/gh/SKILL.md b/skills/gh/SKILL.md index 65b3479dce8..626a41080eb 100644 --- a/skills/gh/SKILL.md +++ b/skills/gh/SKILL.md @@ -160,6 +160,8 @@ Sometimes useful data isn't on the typed commands. Examples: - `gh pr checkout ` switches branches. Use `gh pr diff ` or `gh pr view ` if you only need to read. +- `gh pr checkout --worktree ` checks the PR out into a git worktree + at `` instead of switching the current branch. - `NO_COLOR`, `CLICOLOR_FORCE`, and `GH_FORCE_TTY` are honored. Set `GH_FORCE_TTY=1` if you want TTY-style output (colors, tables, the pager, interactivity) inside an agent harness; leave it unset unless needed. From cd635d803d2d1bcf193a520c6d3a7313a415264b Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Tue, 4 Aug 2026 11:17:52 +0100 Subject: [PATCH 23/25] docs(pr/checkout): add worktree usage example to help text Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6e4cb568-27f3-4c7e-a344-859e2c2d69b0 --- pkg/cmd/pr/checkout/checkout.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/cmd/pr/checkout/checkout.go b/pkg/cmd/pr/checkout/checkout.go index 47a8ece933a..1d6b33a9b5c 100644 --- a/pkg/cmd/pr/checkout/checkout.go +++ b/pkg/cmd/pr/checkout/checkout.go @@ -60,6 +60,7 @@ func NewCmdCheckout(f *cmdutil.Factory, runF func(*CheckoutOptions) error) *cobr $ gh pr checkout 32 $ gh pr checkout https://github.com/OWNER/REPO/pull/32 $ gh pr checkout feature + $ gh pr checkout 32 --branch feature --worktree /path/to/wt-feature `), Args: cobra.MaximumNArgs(1), Aliases: []string{"co"}, From 5e6aa5ab275ad25a8999871f7f86aeda6349099b Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Tue, 4 Aug 2026 11:31:21 +0100 Subject: [PATCH 24/25] fix(pr/checkout): pass -- before worktree path so dash paths work git worktree add treats a path that starts with a hyphen as an option, so a --worktree value like -foo failed with an unknown-switch error. Pass a -- end-of-options separator before the path in every worktree add invocation so the value is always parsed positionally. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6e4cb568-27f3-4c7e-a344-859e2c2d69b0 --- pkg/cmd/pr/checkout/checkout.go | 8 ++++---- pkg/cmd/pr/checkout/checkout_test.go | 12 ++++++------ 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/pkg/cmd/pr/checkout/checkout.go b/pkg/cmd/pr/checkout/checkout.go index 1d6b33a9b5c..962c17de9b8 100644 --- a/pkg/cmd/pr/checkout/checkout.go +++ b/pkg/cmd/pr/checkout/checkout.go @@ -233,10 +233,10 @@ func cmdsForExistingRemote(remote *cliContext.Remote, pr *api.PullRequest, opts cmds = append(cmds, []string{"-C", opts.Worktree, "checkout", "-b", localBranch, "--track", remoteBranch}) } } else if localBranchExists(opts.GitClient, localBranch) { - cmds = append(cmds, []string{"worktree", "add", opts.Worktree, localBranch}) + cmds = append(cmds, []string{"worktree", "add", "--", opts.Worktree, localBranch}) cmds = append(cmds, syncBranchCmds(opts.Worktree, remoteBranchRef, opts.Force)...) } else { - cmds = append(cmds, []string{"worktree", "add", "--track", "-b", localBranch, opts.Worktree, remoteBranch}) + cmds = append(cmds, []string{"worktree", "add", "--track", "-b", localBranch, "--", opts.Worktree, remoteBranch}) } case localBranchExists(opts.GitClient, localBranch): cmds = append(cmds, []string{"checkout", localBranch}) @@ -283,7 +283,7 @@ func cmdsForMissingRemote(pr *api.PullRequest, baseURLOrName, repoHost, defaultB fetchCmd = append(fetchCmd, "--force") } cmds = append(cmds, fetchCmd) - cmds = append(cmds, []string{"worktree", "add", opts.Worktree, localBranch}) + cmds = append(cmds, []string{"worktree", "add", "--", opts.Worktree, localBranch}) } } else if localBranch == currentBranch { // PR head matches currently checked out branch @@ -407,7 +407,7 @@ func detachCmds(fetchCmd []string, worktree string, reuseWorktree bool) [][]stri } return [][]string{ fetchCmd, - {"worktree", "add", "--detach", worktree, "FETCH_HEAD"}, + {"worktree", "add", "--detach", "--", worktree, "FETCH_HEAD"}, } } diff --git a/pkg/cmd/pr/checkout/checkout_test.go b/pkg/cmd/pr/checkout/checkout_test.go index 55369606abc..dd2b5ef644b 100644 --- a/pkg/cmd/pr/checkout/checkout_test.go +++ b/pkg/cmd/pr/checkout/checkout_test.go @@ -336,7 +336,7 @@ func Test_checkoutRun(t *testing.T) { cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "") cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") - cs.Register(`git worktree add --track -b feature /path/to/wt origin/feature`, 0, "") + cs.Register(`git worktree add --track -b feature -- /path/to/wt origin/feature`, 0, "") }, wantStderr: "✓ Checked out PR #123 in worktree /path/to/wt\n To start working: cd /path/to/wt\n", }, @@ -368,7 +368,7 @@ func Test_checkoutRun(t *testing.T) { cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "") cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") - cs.Register(`git worktree add --track -b feature /path/to/wt origin/feature`, 0, "") + cs.Register(`git worktree add --track -b feature -- /path/to/wt origin/feature`, 0, "") cs.Register(`git submodule sync --recursive`, 0, "") cs.Register(`git submodule update --init --recursive`, 0, "") }, @@ -400,7 +400,7 @@ func Test_checkoutRun(t *testing.T) { cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") - cs.Register(`git worktree add /path/to/wt feature`, 0, "") + cs.Register(`git worktree add -- /path/to/wt feature`, 0, "") cs.Register(`git -C /path/to/wt merge --ff-only refs/remotes/origin/feature`, 0, "") }, }, @@ -431,7 +431,7 @@ func Test_checkoutRun(t *testing.T) { cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") - cs.Register(`git worktree add /path/to/wt feature`, 0, "") + cs.Register(`git worktree add -- /path/to/wt feature`, 0, "") cs.Register(`git -C /path/to/wt reset --hard refs/remotes/origin/feature`, 0, "") }, }, @@ -461,7 +461,7 @@ func Test_checkoutRun(t *testing.T) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") cs.Register(`git fetch origin \+refs/heads/feature --no-tags`, 0, "") - cs.Register(`git worktree add --detach /path/to/wt FETCH_HEAD`, 0, "") + cs.Register(`git worktree add --detach -- /path/to/wt FETCH_HEAD`, 0, "") }, }, { @@ -520,7 +520,7 @@ func Test_checkoutRun(t *testing.T) { cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") cs.Register(`git config branch\.feature\.merge`, 1, "") cs.Register(`git fetch origin refs/pull/123/head:feature --no-tags`, 0, "") - cs.Register(`git worktree add /path/to/wt feature`, 0, "") + cs.Register(`git worktree add -- /path/to/wt feature`, 0, "") cs.Register(`git config branch\.feature\.remote https://github.com/hubot/REPO.git`, 0, "") cs.Register(`git config branch\.feature\.pushRemote https://github.com/hubot/REPO.git`, 0, "") cs.Register(`git config branch\.feature\.merge refs/heads/feature`, 0, "") From 980366b4a8cc8374fd6a6bf82903c5bd6e576806 Mon Sep 17 00:00:00 2001 From: tidy-dev <75402236+tidy-dev@users.noreply.github.com> Date: Tue, 4 Aug 2026 07:10:30 -0400 Subject: [PATCH 25/25] Match worktree rev-parse stub against absolute path on Windows The checkoutRun test stubs hardcoded `git -C /path/to/wt rev-parse`, but the command runs after filepath.Abs, which yields `D:\path\to\wt` on Windows and left the stub unmatched (panic: no exec stub). Use a separator-agnostic `.+path.to.wt` pattern so the stub matches the absolute path on every platform while still asserting the -C directory. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/pr/checkout/checkout_test.go | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/pkg/cmd/pr/checkout/checkout_test.go b/pkg/cmd/pr/checkout/checkout_test.go index dd2b5ef644b..73beaa6dcb3 100644 --- a/pkg/cmd/pr/checkout/checkout_test.go +++ b/pkg/cmd/pr/checkout/checkout_test.go @@ -333,7 +333,7 @@ func Test_checkoutRun(t *testing.T) { stdoutTTY: true, runStubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") - cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") + cs.Register(`git -C .+path.to.wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "") cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") cs.Register(`git worktree add --track -b feature -- /path/to/wt origin/feature`, 0, "") @@ -365,7 +365,7 @@ func Test_checkoutRun(t *testing.T) { stdoutTTY: true, runStubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") - cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") + cs.Register(`git -C .+path.to.wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "") cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") cs.Register(`git worktree add --track -b feature -- /path/to/wt origin/feature`, 0, "") @@ -397,7 +397,7 @@ func Test_checkoutRun(t *testing.T) { }, runStubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") - cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") + cs.Register(`git -C .+path.to.wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") cs.Register(`git worktree add -- /path/to/wt feature`, 0, "") @@ -428,7 +428,7 @@ func Test_checkoutRun(t *testing.T) { }, runStubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") - cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") + cs.Register(`git -C .+path.to.wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") cs.Register(`git worktree add -- /path/to/wt feature`, 0, "") @@ -459,7 +459,7 @@ func Test_checkoutRun(t *testing.T) { }, runStubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") - cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") + cs.Register(`git -C .+path.to.wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") cs.Register(`git fetch origin \+refs/heads/feature --no-tags`, 0, "") cs.Register(`git worktree add --detach -- /path/to/wt FETCH_HEAD`, 0, "") }, @@ -488,7 +488,7 @@ func Test_checkoutRun(t *testing.T) { }, runStubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") - cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") + cs.Register(`git -C .+path.to.wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") cs.Register(`git fetch origin \+refs/heads/feature --no-tags`, 0, "") cs.Register(`git -C /path/to/wt checkout --detach FETCH_HEAD`, 0, "") }, @@ -517,7 +517,7 @@ func Test_checkoutRun(t *testing.T) { }, runStubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") - cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") + cs.Register(`git -C .+path.to.wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") cs.Register(`git config branch\.feature\.merge`, 1, "") cs.Register(`git fetch origin refs/pull/123/head:feature --no-tags`, 0, "") cs.Register(`git worktree add -- /path/to/wt feature`, 0, "") @@ -549,7 +549,7 @@ func Test_checkoutRun(t *testing.T) { }, runStubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") - cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") + cs.Register(`git -C .+path.to.wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") cs.Register(`git -C /path/to/wt checkout feature`, 0, "") @@ -581,7 +581,7 @@ func Test_checkoutRun(t *testing.T) { stdoutTTY: true, runStubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") - cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") + cs.Register(`git -C .+path.to.wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") cs.Register(`git show-ref --verify -- refs/heads/my-custom-name`, 1, "") cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") cs.Register(`git -C /path/to/wt checkout -b my-custom-name --track origin/feature`, 0, "") @@ -612,7 +612,7 @@ func Test_checkoutRun(t *testing.T) { }, runStubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") - cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") + cs.Register(`git -C .+path.to.wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") cs.Register(`git config branch\.feature\.merge`, 0, "refs/heads/feature") cs.Register(`git fetch origin refs/pull/123/head --no-tags`, 0, "") @@ -645,7 +645,7 @@ func Test_checkoutRun(t *testing.T) { }, runStubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") - cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") + cs.Register(`git -C .+path.to.wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") cs.Register(`git config branch\.feature\.merge`, 0, "refs/heads/feature") cs.Register(`git fetch origin refs/pull/123/head --no-tags`, 0, "") @@ -677,7 +677,7 @@ func Test_checkoutRun(t *testing.T) { }, runStubs: func(cs *run.CommandStubber) { cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") - cs.Register(`git -C /path/to/wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") + cs.Register(`git -C .+path.to.wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "") cs.Register(`git config branch\.feature\.merge`, 1, "") cs.Register(`git fetch origin refs/pull/123/head --no-tags`, 0, "")