fix: pr-closed-lookup (follow gh: find closed and merged pull requests by branch)

Signed-off-by: bircni <bircni@icloud.com>
This commit is contained in:
bircni committed 2026-10-18 18:57:43 +02:00
1 parent 9ce014bc75
commit 0db9ed463e
9 files changed
+225 -16

No files matched your search

+1 -1
View File
@@ -29,7 +29,7 @@ tea exits with 8 while checks are pending and with 1 when a check failed, like g
if interval <= 0 || timeout <= 0 { if interval <= 0 || timeout <= 0 {
return clierr.Wrap(clierr.Usage, errors.New("--interval and --timeout must be positive")) return clierr.Wrap(clierr.Usage, errors.New("--interval and --timeout must be positive"))
} }
t, n, err := a.pullSelector(cmd, args) t, n, err := a.pullSelectorAny(cmd, args)
if err != nil { if err != nil {
return err return err
} }
+1 -1
View File
@@ -20,7 +20,7 @@ import (
func (a *application) pullCleanCommand() *cobra.Command { func (a *application) pullCleanCommand() *cobra.Command {
var ignoreSHA bool var ignoreSHA bool
cmd := &cobra.Command{Use: "clean [<number> | <url> | <branch>]", Short: "Delete the branches of a merged or closed pull request", Annotations: pullArgumentsAnnotation(map[string]string{"tea.confirm": "true"}), Args: usageArgs(cobra.MaximumNArgs(1)), RunE: func(cmd *cobra.Command, args []string) error { cmd := &cobra.Command{Use: "clean [<number> | <url> | <branch>]", Short: "Delete the branches of a merged or closed pull request", Annotations: pullArgumentsAnnotation(map[string]string{"tea.confirm": "true"}), Args: usageArgs(cobra.MaximumNArgs(1)), RunE: func(cmd *cobra.Command, args []string) error {
target, n, err := a.pullSelector(cmd, args) target, n, err := a.pullSelectorAny(cmd, args)
if err != nil { if err != nil {
return err return err
} }
+133
View File
@@ -0,0 +1,133 @@
// Copyright 2026 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package app
import (
"context"
"fmt"
"net/http"
"strings"
"testing"
"github.com/stretchr/testify/require"
)
// closedLookupServer has one open pull request, for feat, and closed ones for gone, listed
// newest first like sort=recentupdate.
func closedLookupServer(t *testing.T, requests *[]string) string {
t.Helper()
server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
*requests = append(*requests, r.Method+" "+r.URL.Path+"?state="+r.URL.Query().Get("state"))
if number, ok := strings.CutPrefix(r.URL.Path, "/api/v1/repos/alice/project/pulls/"); ok {
_, _ = fmt.Fprintf(w, `{"number":%s,"title":"Old","state":"closed","merged":true,"head":{"ref":"gone","sha":"abc","repo":{"full_name":"alice/project"}}}`, number)
return
}
if r.URL.Path == "/api/v1/repos/alice/project" {
_, _ = fmt.Fprint(w, `{"full_name":"alice/project","default_branch":"main"}`)
return
}
if r.URL.Path != "/api/v1/repos/alice/project/pulls" {
http.NotFound(w, r)
return
}
if r.URL.Query().Get("state") == "open" {
writeFirstPage(w, r, `[{"number":3,"head":{"ref":"feat","repo":{"full_name":"alice/project"}}}]`)
return
}
require.Equal(t, "closed", r.URL.Query().Get("state"))
require.Equal(t, "recentupdate", r.URL.Query().Get("sort"))
writeFirstPage(w, r, `[
{"number":40,"head":{"ref":"gone","repo":{"full_name":"bob/project"}}},
{"number":31,"head":{"ref":"gone","repo":{"full_name":"alice/project"}}},
{"number":30,"head":{"ref":"gone","repo":{"full_name":"alice/project"}}}
]`)
}))
t.Cleanup(server.Close)
return server.URL
}
func TestPullSelectorAnyFallsBackToTheNewestClosedPull(t *testing.T) {
var requests []string
serverURL := closedLookupServer(t, &requests)
for _, tc := range []struct {
name string
arg string
want int64
err string
}{
{"open one wins", "feat", 3, ""},
{"newest closed", "gone", 40, ""},
{"owner qualified", "alice:gone", 31, ""},
{"unknown", "nothing", 0, `no open pull request for branch "nothing"`},
} {
t.Run(tc.name, func(t *testing.T) {
a, _ := refApplicationWith(t, serverURL, nil)
_, n, err := a.pullSelectorAny(commandContext(), []string{tc.arg})
if tc.err != "" {
require.ErrorContains(t, err, tc.err)
return
}
require.NoError(t, err)
require.Equal(t, tc.want, n)
})
}
// Commands that need an open pull request still do not see closed ones.
requests = nil
a, _ := refApplicationWith(t, serverURL, nil)
_, _, err := a.pullSelector(commandContext(), []string{"gone"})
require.ErrorContains(t, err, `no open pull request for branch "gone"`)
require.NotContains(t, strings.Join(requests, "\n"), "state=closed")
}
func TestPullSelectorAnyFallsBackForTheCheckedOutBranch(t *testing.T) {
var requests []string
serverURL := closedLookupServer(t, &requests)
a, _ := refApplicationWith(t, serverURL, map[string]string{"symbolic-ref --quiet HEAD": "gone"})
// The fork's newer pull request for a branch named like the local one is not this repository's.
_, n, err := a.pullSelectorAny(commandContext(), nil)
require.NoError(t, err)
require.Equal(t, int64(31), n)
_, _, err = a.pullSelector(commandContext(), nil)
require.ErrorContains(t, err, `no open pull request for branch "gone"`)
}
func TestPullCommandsFindClosedPullsByBranch(t *testing.T) {
for _, tc := range []struct {
name string
args []string
err string
}{
{"view", []string{"pr", "view", "alice:gone", "--json=number"}, ""},
{"checks", []string{"pr", "checks", "alice:gone", "--web", "--no-input"}, ""},
{"reopen", []string{"pr", "reopen", "alice:gone"}, "already merged"},
{"clean", []string{"pr", "clean", "alice:gone", "--no-input"}, ""},
} {
t.Run(tc.name, func(t *testing.T) {
var requests []string
serverURL := closedLookupServer(t, &requests)
deps, _, _ := serverDeps(t, serverURL)
deps.Getenv = serverEnv(serverURL, map[string]string{"TEA_REPO": "alice/project"})
deps.Git = &fakeGit{results: map[string]string{}}
err := Execute(context.Background(), deps, tc.args)
if tc.err != "" {
require.ErrorContains(t, err, tc.err)
}
if err != nil {
require.NotContains(t, err.Error(), "no open pull request")
}
require.Contains(t, requests, "GET /api/v1/repos/alice/project/pulls/31?state=", "err: %v", err)
})
}
}
func TestPullMergeKeepsOpenOnlyLookup(t *testing.T) {
var requests []string
serverURL := closedLookupServer(t, &requests)
deps, _, _ := serverDeps(t, serverURL)
deps.Getenv = serverEnv(serverURL, map[string]string{"TEA_REPO": "alice/project"})
deps.Git = &fakeGit{results: map[string]string{}}
err := Execute(context.Background(), deps, []string{"pr", "merge", "gone", "--yes", "--no-input"})
require.ErrorContains(t, err, `no open pull request for branch "gone"`)
require.NotContains(t, strings.Join(requests, "\n"), "state=closed")
}
+2 -2
View File
@@ -79,7 +79,7 @@ func (a *application) pullStateCommand(closing bool) *cobra.Command {
var comment string var comment string
var deleteBranch bool var deleteBranch bool
cmd := &cobra.Command{Use: verb + " [<number> | <url> | <branch>]", Short: titleCase(verb) + " a pull request", Annotations: pullArgumentsAnnotation(nil), Args: usageArgs(cobra.MaximumNArgs(1)), RunE: func(cmd *cobra.Command, args []string) error { cmd := &cobra.Command{Use: verb + " [<number> | <url> | <branch>]", Short: titleCase(verb) + " a pull request", Annotations: pullArgumentsAnnotation(nil), Args: usageArgs(cobra.MaximumNArgs(1)), RunE: func(cmd *cobra.Command, args []string) error {
t, n, err := a.pullSelector(cmd, args) t, n, err := a.pullSelectorAny(cmd, args)
if err != nil { if err != nil {
return err return err
} }
@@ -176,7 +176,7 @@ func (a *application) pullDiffCommand() *cobra.Command {
On a terminal the diff is colored and paged. Piped, it is written unchanged, so that On a terminal the diff is colored and paged. Piped, it is written unchanged, so that
tea pr diff 13 | git apply works.`, Annotations: pullArgumentsAnnotation(textOutput("diff returns patch text")), Args: usageArgs(cobra.MaximumNArgs(1)), RunE: func(cmd *cobra.Command, args []string) error { tea pr diff 13 | git apply works.`, Annotations: pullArgumentsAnnotation(textOutput("diff returns patch text")), Args: usageArgs(cobra.MaximumNArgs(1)), RunE: func(cmd *cobra.Command, args []string) error {
t, n, err := a.pullSelector(cmd, args) t, n, err := a.pullSelectorAny(cmd, args)
if err != nil { if err != nil {
return err return err
} }
+1 -1
View File
@@ -150,7 +150,7 @@ func (a *application) pullViewCommand() *cobra.Command {
if comments && a.options.json != "" { if comments && a.options.json != "" {
return clierr.Wrap(clierr.Usage, errors.New("--comments cannot be combined with --json or --jq; use tea pr comment list <number> --json or tea pr review list <number> --json")) return clierr.Wrap(clierr.Usage, errors.New("--comments cannot be combined with --json or --jq; use tea pr comment list <number> --json or tea pr review list <number> --json"))
} }
t, n, err := a.pullSelector(cmd, args) t, n, err := a.pullSelectorAny(cmd, args)
if err != nil { if err != nil {
return err return err
} }
+54 -4
View File
@@ -196,13 +196,26 @@ func (a *application) sameRepository(first, second string) bool {
// pullSelector resolves the pull request a pr command addresses: N, #N, OWNER/REPO#N, a pull // pullSelector resolves the pull request a pr command addresses: N, #N, OWNER/REPO#N, a pull
// request URL, a head branch, OWNER:BRANCH, or, without an argument, the pull request of the // request URL, a head branch, OWNER:BRANCH, or, without an argument, the pull request of the
// checked-out branch. // checked-out branch.
//
// A branch names an open pull request only; commands that mutate open pull requests use it
// as is, and the others use pullSelectorAny.
func (a *application) pullSelector(cmd *cobra.Command, args []string) (target, int64, error) { func (a *application) pullSelector(cmd *cobra.Command, args []string) (target, int64, error) {
return a.selectPull(cmd, args, false)
}
// pullSelectorAny is pullSelector that, when no open pull request has the branch, falls back
// to the most recently updated closed or merged one, like gh.
func (a *application) pullSelectorAny(cmd *cobra.Command, args []string) (target, int64, error) {
return a.selectPull(cmd, args, true)
}
func (a *application) selectPull(cmd *cobra.Command, args []string, closed bool) (target, int64, error) {
if len(args) == 0 || args[0] == "" { if len(args) == 0 || args[0] == "" {
// The checked-out branch says nothing about a repository chosen with --repo. // The checked-out branch says nothing about a repository chosen with --repo.
if a.options.repo != "" { if a.options.repo != "" {
return target{}, 0, clierr.Wrap(clierr.Usage, errors.New("pass a pull request number, URL or branch with --repo")) return target{}, 0, clierr.Wrap(clierr.Usage, errors.New("pass a pull request number, URL or branch with --repo"))
} }
return a.currentPull(cmd.Context()) return a.currentPull(cmd.Context(), closed)
} }
raw := args[0] raw := args[0]
if _, err := parseRef(raw, pullRef); !errors.Is(err, errNotRef) { if _, err := parseRef(raw, pullRef); !errors.Is(err, errNotRef) {
@@ -216,7 +229,8 @@ func (a *application) pullSelector(cmd *cobra.Command, args []string) (target, i
if !qualified { if !qualified {
owner, branch = "", raw owner, branch = "", raw
} }
pulls, err := (services.Pulls{Client: t.client, Repository: t.Repo}).ListOpen(cmd.Context()) service := services.Pulls{Client: t.client, Repository: t.Repo}
pulls, err := service.ListOpen(cmd.Context())
if err != nil { if err != nil {
return t, 0, err return t, 0, err
} }
@@ -224,6 +238,14 @@ func (a *application) pullSelector(cmd *cobra.Command, args []string) (target, i
if err != nil { if err != nil {
return t, 0, err return t, 0, err
} }
if n == 0 && closed {
if n, err = newestClosedForBranch(cmd.Context(), service, owner, branch, false, t.Repo); err != nil {
return t, 0, err
}
if n > 0 {
return t, n, nil
}
}
if n == 0 { if n == 0 {
return t, 0, fmt.Errorf("no open pull request for branch %q in %s; pass a number, URL or branch", raw, t.Repo) return t, 0, fmt.Errorf("no open pull request for branch %q in %s; pass a number, URL or branch", raw, t.Repo)
} }
@@ -233,8 +255,9 @@ func (a *application) pullSelector(cmd *cobra.Command, args []string) (target, i
// currentPull finds the pull request of the checked-out branch: the one pr checkout recorded // 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 // while it is open, else the open pull request whose head is the branch's upstream (and the
// upstream remote's owner), else one of this repository whose head has the local branch's name, else N from a pr/N // upstream remote's owner), else one of this repository 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. // branch, else the recorded pull request even though it is no longer open. With closed, the
func (a *application) currentPull(ctx context.Context) (target, int64, error) { // newest closed or merged pull request for the branch comes last.
func (a *application) currentPull(ctx context.Context, closed bool) (target, int64, error) {
branch, err := git.CurrentBranchConfig(ctx, a.gitProbe(), a.options.cwd) branch, err := git.CurrentBranchConfig(ctx, a.gitProbe(), a.options.cwd)
if err != nil { if err != nil {
return target{}, 0, clierr.Wrap(clierr.Usage, errors.New("not on a branch; pass a pull request number, URL or branch")) return target{}, 0, clierr.Wrap(clierr.Usage, errors.New("not on a branch; pass a pull request number, URL or branch"))
@@ -256,6 +279,7 @@ func (a *application) currentPull(ctx context.Context) (target, int64, error) {
if err != nil { if err != nil {
return t, 0, err return t, 0, err
} }
upstreamOwner := ""
if branch.Merge != "" { if branch.Merge != "" {
owner := "" owner := ""
if remoteURL, err := a.gitProbe().Output(ctx, a.options.cwd, false, "remote", "get-url", branch.Remote); err == nil { if remoteURL, err := a.gitProbe().Output(ctx, a.options.cwd, false, "remote", "get-url", branch.Remote); err == nil {
@@ -266,6 +290,7 @@ func (a *application) currentPull(ctx context.Context) (target, int64, error) {
if n, err := pullForBranch(pulls, owner, branch.Merge, t.Repo); owner != "" && (n > 0 || err != nil) { if n, err := pullForBranch(pulls, owner, branch.Merge, t.Repo); owner != "" && (n > 0 || err != nil) {
return t, n, err return t, n, err
} }
upstreamOwner = owner
} }
// A bare branch name only identifies the repository's own pull requests: a contributor's // A bare branch name only identifies the repository's own pull requests: a contributor's
// fork may use main or fix as its head branch too. // fork may use main or fix as its head branch too.
@@ -281,9 +306,34 @@ func (a *application) currentPull(ctx context.Context) (target, int64, error) {
if recorded > 0 { if recorded > 0 {
return t, recorded, nil return t, recorded, nil
} }
if closed {
// Same matching as above: the upstream's owner first, then the repository's own branches.
if upstreamOwner != "" {
if n, err := newestClosedForBranch(ctx, service, upstreamOwner, branch.Merge, false, t.Repo); n > 0 || err != nil {
return t, n, err
}
}
if n, err := newestClosedForBranch(ctx, service, "", branch.Name, true, t.Repo); n > 0 || err != nil {
return t, n, err
}
}
return t, 0, fmt.Errorf("no open pull request for branch %q in %s; pass a number, URL or branch", branch.Name, t.Repo) return t, 0, fmt.Errorf("no open pull request for branch %q in %s; pass a number, URL or branch", branch.Name, t.Repo)
} }
// newestClosedForBranch returns the number of the most recently updated closed or merged pull
// request whose head is branch in a repository of owner (any owner when empty), or 0. With own,
// only pull requests from the repository itself count, as a fork may reuse the branch name.
func newestClosedForBranch(ctx context.Context, service services.Pulls, owner, branch string, own bool, repo string) (int64, error) {
pull, found, err := service.NewestClosed(ctx, func(pull services.Pull) bool {
headOwner, _, _ := strings.Cut(pull.HeadRepo, "/")
return pull.Head == branch && (owner == "" || strings.EqualFold(headOwner, owner)) && (!own || strings.EqualFold(pull.HeadRepo, repo))
})
if err != nil || !found {
return 0, err
}
return pull.Number, nil
}
// remoteOwns reports whether the Git remote named remote points at the target's repository. // remoteOwns reports whether the Git remote named remote points at the target's repository.
func (a *application) remoteOwns(ctx context.Context, t target, remote string) bool { func (a *application) remoteOwns(ctx context.Context, t target, remote string) bool {
raw, err := a.gitProbe().Output(ctx, a.options.cwd, false, "remote", "get-url", remote) raw, err := a.gitProbe().Output(ctx, a.options.cwd, false, "remote", "get-url", remote)
+12 -6
View File
@@ -82,19 +82,25 @@ func refApplication(t *testing.T, gitResults map[string]string) (*application, s
]`) ]`)
})) }))
t.Cleanup(server.Close) t.Cleanup(server.Close)
return refApplicationWith(t, server.URL, gitResults)
}
// refApplicationWith is refApplication for an existing server.
func refApplicationWith(t *testing.T, serverURL string, gitResults map[string]string) (*application, string) {
t.Helper()
deps, _, _ := testDeps(t) deps, _, _ := testDeps(t)
deps.Getenv = serverEnv(server.URL, map[string]string{"TEA_REPO": "alice/project"}) deps.Getenv = serverEnv(serverURL, map[string]string{"TEA_REPO": "alice/project"})
results := map[string]string{ results := map[string]string{
"remote get-url origin": server.URL + "/alice/project.git", "remote get-url origin": serverURL + "/alice/project.git",
"remote get-url pulls/bob/project": server.URL + "/bob/project.git", "remote get-url pulls/bob/project": serverURL + "/bob/project.git",
"remote get-url fork": server.URL + "/bob/project.git", "remote get-url fork": serverURL + "/bob/project.git",
"remote get-url pulls/carol/project": server.URL + "/carol/project.git", "remote get-url pulls/carol/project": serverURL + "/carol/project.git",
} }
for key, value := range gitResults { for key, value := range gitResults {
results[key] = value results[key] = value
} }
deps.Git = &fakeGit{results: results} deps.Git = &fakeGit{results: results}
return &application{deps: deps}, server.URL return &application{deps: deps}, serverURL
} }
func commandContext() *cobra.Command { func commandContext() *cobra.Command {
+1 -1
View File
@@ -96,7 +96,7 @@ func (a *application) currentPullStatus(cmd *cobra.Command) (*services.Pull, pul
if err != nil { if err != nil {
return nil, details, "" return nil, details, ""
} }
t, n, err := a.pullSelector(cmd, nil) t, n, err := a.pullSelectorAny(cmd, nil)
if err != nil { if err != nil {
return nil, details, branch return nil, details, branch
} }
+20
View File
@@ -276,6 +276,26 @@ func (s Pulls) ListOpen(ctx context.Context) ([]Pull, error) {
}) })
} }
// NewestClosed returns the most recently updated closed or merged pull request that keep
// accepts, and whether there is one. It reads pages newest first and stops at the first match.
func (s Pulls) NewestClosed(ctx context.Context, keep func(Pull) bool) (Pull, bool, error) {
for page := 1; page <= maxListPages; page++ {
items, _, err := s.List(ctx, PullListOptions{State: "closed", Sort: "recentupdate", Limit: 50, Page: page})
if err != nil {
return Pull{}, false, err
}
if len(items) == 0 {
return Pull{}, false, nil
}
for _, item := range items {
if keep(item) {
return item, true, nil
}
}
}
return Pull{}, false, nil
}
// Create opens a pull request. // Create opens a pull request.
func (s Pulls) Create(ctx context.Context, opts PullOptions) (Pull, error) { func (s Pulls) Create(ctx context.Context, opts PullOptions) (Pull, error) {
// TODO(sdk): CreatePullRequestOption lacks allow_maintainer_edit. // TODO(sdk): CreatePullRequestOption lacks allow_maintainer_edit.