fix: tighten error classification and pull request selection

Only Gitea's archived-repository messages mean the repository is archived,
so an archived label is no longer reported as one. Refused POSTs to the
organization, team, user and SSH key collections name the resource they would
have created, and the existing pull request is looked up with escaped branch
names.

pullSelector without an argument refuses an explicit --repo, like gh: the
checked-out branch says nothing about another repository.

Signed-off-by: bircni <bircni@icloud.com>

internal/app/args.go

@@ -295,10 +295,11 @@ func argumentName(cmd *cobra.Command, argument string) string {
 	if group == cmd {
 		return text
 	}
-	if noun := group.Name(); noun == "pr" {
+	noun := group.Name()
-		return "pull request " + text
+	if noun == "pr" {
+		noun = "pull request"
 	}
-	return group.Name() + " " + text
+	return noun + " " + text
 }
 
 // localFlagsChanged reports whether any command-specific flag was supplied.

internal/app/download.go

@@ -5,6 +5,7 @@ package app
 
 import (
 	"bufio"
+	"cmp"
 	"errors"
 	"fmt"
 	"io"
@@ -58,10 +59,10 @@ func (f downloadFlags) destination(name string) (path string, skip bool, err err
 	}
 	path = f.output
 	if path == "" || isDirectory(path) {
-		if name != filepath.Base(name) || name == "." || name == ".." || strings.ContainsAny(name, `/\`) {
+		if name == "" || name == "." || name == ".." || strings.ContainsAny(name, `/\`) {
 			return "", false, clierr.Wrap(clierr.Usage, fmt.Errorf("cannot use %q as a file name; pass --output", name))
 		}
-		path = filepath.Join(cmpOrDot(path, f.dir), name)
+		path = filepath.Join(cmp.Or(path, f.dir, "."), name)
 	}
 	if _, err := os.Lstat(path); err == nil {
 		if f.skipExisting {
@@ -76,15 +77,6 @@ func (f downloadFlags) destination(name string) (path string, skip bool, err err
 	return path, false, nil
 }
 
-func cmpOrDot(values ...string) string {
-	for _, value := range values {
-		if value != "" {
-			return value
-		}
-	}
-	return "."
-}
-
 func isDirectory(path string) bool {
 	info, err := os.Stat(path)
 	return (err == nil && info.IsDir()) || strings.HasSuffix(path, string(filepath.Separator)) || strings.HasSuffix(path, "/")

internal/app/errors.go

@@ -10,6 +10,7 @@ import (
 	"fmt"
 	"net"
 	"net/http"
+	"net/url"
 	"strings"
 	"syscall"
 	"time"
@@ -206,7 +207,7 @@ func existsHint(ctx context.Context, apiErr *transport.APIError, conflict *servi
 		var pull struct {
 			URL string `json:"html_url"`
 		}
-		if apiErr.Lookup(ctx, "repos/"+conflict.Scope+"/pulls/"+base+"/"+head, &pull) == http.StatusOK && pull.URL != "" {
+		if apiErr.Lookup(ctx, "repos/"+conflict.Scope+"/pulls/"+url.PathEscape(base)+"/"+url.PathEscape(head), &pull) == http.StatusOK && pull.URL != "" {
 			return "see " + pull.URL
 		}
 	case "wiki page":

internal/app/ref.go

@@ -74,11 +74,14 @@ func parseRefURL(raw string, kind refKind) (ref, error) {
 		return ref{}, invalid
 	}
 	segments := strings.Split(strings.Trim(u.Path, "/"), "/")
-	positive := func(i int) int64 {
+	at := func(i int) string {
-		if i >= len(segments) {
+		if i < len(segments) {
-			return 0
+			return segments[i]
 		}
-		n, err := strconv.ParseInt(segments[i], 10, 64)
+		return ""
+	}
+	positive := func(i int) int64 {
+		n, err := strconv.ParseInt(at(i), 10, 64)
 		if err != nil || n < 1 {
 			return 0
 		}
@@ -88,12 +91,12 @@ func parseRefURL(raw string, kind refKind) (ref, error) {
 	for i := 2; i < len(segments); i++ {
 		result := ref{}
 		switch {
-		case kind == runRef && segments[i] == "actions" && i+2 < len(segments) && segments[i+1] == "runs" && positive(i+2) > 0:
+		case kind == runRef && at(i) == "actions" && at(i+1) == "runs" && positive(i+2) > 0:
 			result.Number = positive(i + 2)
-			if i+4 < len(segments) && segments[i+3] == "jobs" {
+			if at(i+3) == "jobs" {
 				result.Job = positive(i + 4)
 			}
-		case kind != runRef && (segments[i] == "issues" || segments[i] == "pulls") && positive(i+1) > 0:
+		case kind != runRef && (at(i) == "issues" || at(i) == "pulls") && positive(i+1) > 0:
 			result.Number = positive(i + 1)
 		default:
 			continue
@@ -188,6 +191,10 @@ func (a *application) sameRepository(first, second string) bool {
 // checked-out branch.
 func (a *application) pullSelector(cmd *cobra.Command, args []string) (target, int64, error) {
 	if len(args) == 0 || args[0] == "" {
+		// The checked-out branch says nothing about a repository chosen with --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 a.currentPull(cmd.Context())
 	}
 	raw := args[0]

internal/app/ref_test.go

@@ -135,6 +135,11 @@ func TestPullSelectorDefaultsToTheCheckedOutBranch(t *testing.T) {
 			require.Equal(t, tc.want, n)
 		})
 	}
+	a, _ := refApplication(t, branch("feat/two", nil))
+	a.options.repo = "alice/project"
+	_, _, err := a.pullSelector(commandContext(), nil)
+	require.Equal(t, clierr.Usage, clierr.ExitCode(err))
+	require.EqualError(t, err, "pass a pull request number, URL or branch with --repo")
 }
 
 func TestPullSelectorAcceptsNumbersURLsAndBranches(t *testing.T) {

internal/services/errors.go

@@ -141,7 +141,7 @@ var refusals = []struct {
 	statuses  []int
 	fragments []string
 }{
-	{ReasonArchived, nil, []string{"is archived"}},
+	{ReasonArchived, nil, []string{"repo is archived", "repository is archived", "> is archived"}},
 	{ReasonMerged, nil, []string{"already merged"}},
 	{ReasonDraft, []int{http.StatusMethodNotAllowed}, []string{"work in progress", "draft"}},
 	{ReasonOwnReview, nil, []string{"approve your own pull", "reject your own pull"}},
@@ -341,16 +341,24 @@ func instanceResource(parts []string) Resource {
 		return Resource{Kind: "member", Key: strconv.Quote(at(3)), Scope: at(1)}
 	case at(0) == "orgs" && at(1) != "" && at(2) == "repos":
 		return Resource{Kind: "organization", Key: strconv.Quote(at(1)), Collection: "repository"}
+	case at(0) == "orgs" && at(1) != "" && at(2) == "teams":
+		return Resource{Kind: "organization", Key: strconv.Quote(at(1)), Collection: "team"}
 	case at(0) == "orgs" && at(1) != "":
 		return Resource{Kind: "organization", Key: strconv.Quote(at(1))}
+	case at(0) == "orgs":
+		return Resource{Collection: "organization"}
 	case at(0) == "users" && at(1) != "":
 		return Resource{Kind: "user", Key: strconv.Quote(at(1))}
 	case at(0) == "admin" && at(1) == "users" && at(2) != "":
 		return Resource{Kind: "user", Key: strconv.Quote(at(2))}
+	case at(0) == "admin" && at(1) == "users":
+		return Resource{Collection: "user"}
 	case at(0) == "teams" && isNumber(at(1)):
 		return Resource{Kind: "team", Key: at(1)}
 	case at(0) == "user" && at(1) == "keys" && at(2) != "":
 		return Resource{Kind: "SSH key", Key: at(2)}
+	case at(0) == "user" && at(1) == "keys":
+		return Resource{Collection: "SSH key"}
 	case at(0) == "user" && at(1) == "repos":
 		return Resource{Collection: "repository"}
 	case at(0) == "notifications" && at(1) == "threads" && at(2) != "":

internal/services/errors_test.go

@@ -51,6 +51,10 @@ func TestClassifyRequestNamesTheResource(t *testing.T) {
 		{"wiki page exists", "POST", "/api/v1/repos/alice/x/wiki/new", "wiki page already exists [title: Home.md]", 400, `wiki page "Home" already exists in alice/x`},
 		{"repository exists", "POST", "/api/v1/user/repos", "The repository with the same name already exists.", 409, "repository already exists"},
 		{"org repository exists", "POST", "/api/v1/orgs/acme/repos", "The repository with the same name already exists.", 409, "repository already exists"},
+		{"team exists", "POST", "/api/v1/orgs/acme/teams", "team already exists [org_id: 3, name: devs]", 422, "team already exists"},
+		{"user exists", "POST", "/api/v1/admin/users", "user already exists [name: bob]", 422, "user already exists"},
+		{"organization exists", "POST", "/api/v1/orgs", "user already exists [name: acme]", 422, "organization already exists"},
+		{"archived label is no repository", "POST", "/api/v1/repos/alice/x/issues/1/labels", "label is archived", 422, "Gitea API returned HTTP 422 for POST /api/v1/repos/alice/x/issues/1/labels: label is archived"},
 		{"run completed", "POST", "/api/v1/repos/alice/x/actions/runs/35/cancel", "run is already completed", 409, "run 35 is already completed"},
 		{"run not done", "DELETE", "/api/v1/repos/alice/x/actions/runs/37", "this workflow run is not done", 400, "run 37 has not finished"},
 		{"workflow disabled", "POST", "/api/v1/repos/alice/x/actions/workflows/nightly.yaml/dispatches", "workflow is disabled", 403, "workflow nightly.yaml is disabled"},