From badd934d64f1318dd6bba8f435cbfafd73daf627 Mon Sep 17 00:00:00 2001 From: bircni Date: Mon, 12 Oct 2026 20:25:40 +0200 Subject: [PATCH] fix(pr): prefer an open pull request over a merged one recorded by checkout Signed-off-by: bircni --- internal/app/ref.go | 20 +++++++++++++++----- internal/app/ref_test.go | 13 ++++++++++++- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/internal/app/ref.go b/internal/app/ref.go index 0cf7ee8c..201a6bfc 100644 --- a/internal/app/ref.go +++ b/internal/app/ref.go @@ -223,9 +223,10 @@ func (a *application) pullSelector(cmd *cobra.Command, args []string) (target, i return t, n, nil } -// currentPull finds the pull request of the checked-out branch: the one pr checkout recorded, -// else the open pull request whose head is the branch's upstream (and the upstream remote's -// owner), else one whose head has the local branch's name, else N from a pr/N branch. +// currentPull finds the pull request of the checked-out branch: the one pr checkout recorded +// while it is open, else the open pull request whose head is the branch's upstream (and the +// upstream remote's owner), else one whose head has the local branch's name, else N from a pr/N +// branch, else the recorded pull request even though it is no longer open. func (a *application) currentPull(ctx context.Context) (target, int64, error) { branch, err := git.CurrentBranchConfig(ctx, a.gitProbe(), a.options.cwd) if err != nil { @@ -235,10 +236,16 @@ func (a *application) currentPull(ctx context.Context) (target, int64, error) { if err != nil { return t, 0, err } + service := services.Pulls{Client: t.client, Repository: t.Repo} + // A recorded pull request that was merged or closed only counts when no open one has the branch. + recorded := int64(0) if branch.PullNumber > 0 && (branch.BaseRemote == "" || a.remoteOwns(ctx, t, branch.BaseRemote)) { - return t, branch.PullNumber, nil + if pull, err := service.View(ctx, branch.PullNumber); err != nil || pull.State == "open" { + return t, branch.PullNumber, nil + } + recorded = branch.PullNumber } - pulls, err := (services.Pulls{Client: t.client, Repository: t.Repo}).ListOpen(ctx) + pulls, err := service.ListOpen(ctx) if err != nil { return t, 0, err } @@ -261,6 +268,9 @@ func (a *application) currentPull(ctx context.Context) (target, int64, error) { return t, n, nil } } + if recorded > 0 { + return t, recorded, nil + } return t, 0, fmt.Errorf("no open pull request for branch %q in %s; pass a number, URL or branch", branch.Name, t.Repo) } diff --git a/internal/app/ref_test.go b/internal/app/ref_test.go index 33dbe2de..f74c5054 100644 --- a/internal/app/ref_test.go +++ b/internal/app/ref_test.go @@ -6,6 +6,7 @@ package app import ( "context" "net/http" + "strings" "testing" "gitea.dev/tea/internal/clierr" @@ -59,10 +60,18 @@ func TestParseRefAcceptsNumbersQualifiedNumbersAndURLs(t *testing.T) { // refApplication returns an application authenticated against a server listing the open pull // requests #3 (feat/one in alice/project), #4 (feat/two), #17 (fix in bob/project) and two -// pull requests from branch dup. +// pull requests from branch dup. Viewing one pull request finds it open, except the merged #30. func refApplication(t *testing.T, gitResults map[string]string) (*application, string) { t.Helper() server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if number, ok := strings.CutPrefix(r.URL.Path, "/api/v1/repos/alice/project/pulls/"); ok { + state := "open" + if number == "30" { + state = "closed" + } + _, _ = w.Write([]byte(`{"number":` + number + `,"state":"` + state + `","merged":true}`)) + return + } require.Equal(t, "/api/v1/repos/alice/project/pulls", r.URL.Path) writeFirstPage(w, r, `[ {"number":3,"head":{"ref":"feat/one","repo":{"full_name":"alice/project"}}}, @@ -111,6 +120,8 @@ func TestPullSelectorDefaultsToTheCheckedOutBranch(t *testing.T) { }{ {"recorded by pr checkout", branch("pr/17", map[string]string{"tea-pr": "17", "tea-base-remote": "origin", "remote": "pulls/bob/project", "merge": "refs/heads/fix"}), 17, ""}, {"head branch name kept by pr checkout", branch("fix", map[string]string{"tea-pr": "17", "tea-base-remote": "origin"}), 17, ""}, + {"recorded but merged, the head branch has a newer one", branch("fix", map[string]string{"tea-pr": "30", "tea-base-remote": "origin"}), 17, ""}, + {"recorded but merged, no open one", branch("gone", map[string]string{"tea-pr": "30", "tea-base-remote": "origin"}), 30, ""}, {"recorded for another repository", branch("pr/9", map[string]string{"tea-pr": "9", "tea-base-remote": "fork", "remote": "fork", "merge": "refs/heads/fix"}), 17, ""}, {"upstream branch and owner", branch("local", map[string]string{"remote": "origin", "merge": "refs/heads/feat/one"}), 3, ""}, {"upstream owner tells forks apart", branch("dup", map[string]string{"remote": "pulls/carol/project", "merge": "refs/heads/dup"}), 21, ""},