Only find PRs w/ branch.<name>.merge if push.default = upstream/tracking

When push.default is not 'upstream' / 'tracking' (or 'nothing'), we can expect local and remote branch names to be the same and solely rely on @{push} or RemoteURL. This fixes the wrong error message 'no pull requests found for branch "<target branch>"' when the local branch is not pushed in the push.default = simple / current and upstream = <target branch> setup.
cli · jtmcg · Jan 29, 2025 · Jun 14, 2024 · Jun 14, 2024 · Jun 14, 2024
commit 7fc35fd47d9482068a5c7666c5c67d4e5e001fce
@@ -38,6 +38,7 @@ type finder struct {
 	remotesFn    func() (remotes.Remotes, error)
 	httpClient   func() (*http.Client, error)
 	branchConfig func(string) git.BranchConfig
+	pushDefault  func() (string, error)
 	progress     progressIndicator
 
 	repo       ghrepo.Interface
@@ -57,7 +58,10 @@ func NewFinder(factory *cmdutil.Factory) PRFinder {
 		branchFn:   factory.Branch,
 		remotesFn:  factory.Remotes,
 		httpClient: factory.HttpClient,
-		progress:   factory.IOStreams,
+		pushDefault: func() (string, error) {
+			return factory.GitClient.Config(context.Background(), "push.default")
+		},
+		progress: factory.IOStreams,
 		branchConfig: func(s string) git.BranchConfig {
 			return factory.GitClient.ReadBranchConfig(context.Background(), s)
 		},
@@ -263,7 +267,8 @@ func (f *finder) parseCurrentBranch() (string, int, error) {
 	if gitRemoteRepo != nil {
 		if branchConfig.Push != "" {
 			prHeadRef = strings.TrimPrefix(branchConfig.Push, branchConfig.RemoteName+"/")
-		} else if strings.HasPrefix(branchConfig.MergeRef, "refs/heads/") {
+		} else if pushDefault, _ := f.pushDefault(); (pushDefault == "upstream" || pushDefault == "tracking") &&
+			strings.HasPrefix(branchConfig.MergeRef, "refs/heads/") {
 			prHeadRef = strings.TrimPrefix(branchConfig.MergeRef, "refs/heads/")
 		}
 		// prepend `OWNER:` if this branch is pushed to a fork

@@ -18,6 +18,7 @@ func TestFind(t *testing.T) {
 		baseRepoFn   func() (ghrepo.Interface, error)
 		branchFn     func() (string, error)
 		branchConfig func(string) git.BranchConfig
+		pushDefault  func() (string, error)
 		remotesFn    func() (context.Remotes, error)
 		selector     string
 		fields       []string
@@ -330,6 +331,7 @@ func TestFind(t *testing.T) {
 					c.Push = "origin/blue-upstream-berries"
 					return
 				},
+				pushDefault: func() (string, error) { return "upstream", nil },
 				remotesFn: func() (context.Remotes, error) {
 					return context.Remotes{{
 						Remote: &git.Remote{Name: "origin"},
@@ -373,7 +375,52 @@ func TestFind(t *testing.T) {
 					c.RemoteURL = u
 					return
 				},
-				remotesFn: nil,
+				pushDefault: func() (string, error) { return "upstream", nil },
+				remotesFn:   nil,
+			},
+			httpStub: func(r *httpmock.Registry) {
+				r.Register(
+					httpmock.GraphQL(`query PullRequestForBranch\b`),
+					httpmock.StringResponse(`{"data":{"repository":{
+						"pullRequests":{"nodes":[
+							{
+								"number": 13,
+								"state": "OPEN",
+								"baseRefName": "main",
+								"headRefName": "blue-upstream-berries",
+								"isCrossRepository": true,
+								"headRepositoryOwner": {"login":"UPSTREAMOWNER"}
+							}
+						]}
+					}}}`))
+			},
+			wantPR:   13,
+			wantRepo: "https://github.com/OWNER/REPO",
+		},
+		{
+			name: "current branch with tracking (deprecated synonym of upstream) configuration",
+			args: args{
+				selector: "",
+				fields:   []string{"id", "number"},
+				baseRepoFn: func() (ghrepo.Interface, error) {
+					return ghrepo.FromFullName("OWNER/REPO")
+				},
+				branchFn: func() (string, error) {
+					return "blueberries", nil
+				},
+				branchConfig: func(branch string) (c git.BranchConfig) {
+					c.MergeRef = "refs/heads/blue-upstream-berries"
+					c.RemoteName = "origin"
+					c.Push = "origin/blue-upstream-berries"
+					return
+				},
+				pushDefault: func() (string, error) { return "tracking", nil },
+				remotesFn: func() (context.Remotes, error) {
+					return context.Remotes{{
+						Remote: &git.Remote{Name: "origin"},
+						Repo:   ghrepo.New("UPSTREAMOWNER", "REPO"),
+					}}, nil
+				},
 			},
 			httpStub: func(r *httpmock.Registry) {
 				r.Register(
@@ -407,6 +454,7 @@ func TestFind(t *testing.T) {
 				},
 				branchConfig: func(branch string) (c git.BranchConfig) {
 					c.RemoteName = "origin"
+					c.Push = "origin/blueberries"
 					return
 				},
 				remotesFn: func() (context.Remotes, error) {
@@ -539,6 +587,7 @@ func TestFind(t *testing.T) {
 				baseRepoFn:   tt.args.baseRepoFn,
 				branchFn:     tt.args.branchFn,
 				branchConfig: tt.args.branchConfig,
+				pushDefault:  tt.args.pushDefault,
 				remotesFn:    tt.args.remotesFn,
 			}
 

@@ -211,7 +211,8 @@ func prSelectorForCurrentBranch(gitClient *git.Client, baseRepo ghrepo.Interface
 	if branchOwner != "" {
 		if branchConfig.Push != "" {
 			selector = strings.TrimPrefix(branchConfig.Push, branchConfig.RemoteName+"/")
-		} else if strings.HasPrefix(branchConfig.MergeRef, "refs/heads/") {
+		} else if pushDefault, _ := gitClient.Config(context.Background(), "push.default"); (pushDefault == "upstream" || pushDefault == "tracking") &&
+			strings.HasPrefix(branchConfig.MergeRef, "refs/heads/") {
 			selector = strings.TrimPrefix(branchConfig.MergeRef, "refs/heads/")
 		}
 		// prepend `OWNER:` if this branch is pushed to a fork

@@ -375,7 +375,7 @@ Requesting a code review from you
 	}
 }
 
-func Test_prSelectorForCurrentBranch(t *testing.T) {
+func Test_prSelectorForCurrentBranchPushDefaultUpstream(t *testing.T) {
 	rs, cleanup := run.Stub()
 	defer cleanup(t)
 
@@ -384,6 +384,38 @@ func Test_prSelectorForCurrentBranch(t *testing.T) {
 		branch.Frederick888/main.merge refs/heads/main
 	`))
 	rs.Register(`git rev-parse --verify --quiet --abbrev-ref Frederick888/main@\{push\}`, 1, "")
+	rs.Register(`git config push\.default`, 0, "upstream")
+
+	repo := ghrepo.NewWithHost("octocat", "playground", "github.com")
+	rem := context.Remotes{
+		&context.Remote{
+			Remote: &git.Remote{Name: "origin"},
+			Repo:   repo,
+		},
+	}
+	gitClient := &git.Client{GitPath: "some/path/git"}
+	prNum, headRef, err := prSelectorForCurrentBranch(gitClient, repo, "Frederick888/main", rem)
+	if err != nil {
+		t.Fatalf("prSelectorForCurrentBranch error: %v", err)
+	}
+	if prNum != 0 {
+		t.Errorf("expected prNum to be 0, got %q", prNum)
+	}
+	if headRef != "Frederick888:main" {
+		t.Errorf("expected headRef to be \"Frederick888:main\", got %q", headRef)
+	}
+}
+
+func Test_prSelectorForCurrentBranchPushDefaultTracking(t *testing.T) {
+	rs, cleanup := run.Stub()
+	defer cleanup(t)
+
+	rs.Register(`git config --get-regexp \^branch\\.`, 0, heredoc.Doc(`
+		branch.Frederick888/main.remote git@github.com:Frederick888/playground.git
+		branch.Frederick888/main.merge refs/heads/main
+	`))
+	rs.Register(`git rev-parse --verify --quiet --abbrev-ref Frederick888/main@\{push\}`, 1, "")
+	rs.Register(`git config push\.default`, 0, "tracking")
 
 	repo := ghrepo.NewWithHost("octocat", "playground", "github.com")
 	rem := context.Remotes{