feat: accept repository shorthands and name instances in account errors

Repository arguments and --repo accept NAME (owned by the active account, or the --sudo user), OWNER/NAME.git and HOST/OWNER/NAME next to URLs; --host, TEA_HOST and GITEA_INSTANCE_URL accept a bare HOST[:PORT][/PATH], which takes the scheme of the configured instance it names, else https. --repo origin in a clone with that remote suggests --remote origin. Account errors name the instance, the accounts that are logged in and where the instance came from, such as "no account for https://gitea.com (instance from git remote \"origin\")", instead of "no selected account". Signed-off-by: bircni <bircni@icloud.com>
2026-10-26 19:11:25 +01:00 · 2026-10-11 22:48:49 +02:00
1 parent c8073f1b2e
commit 2d4d34ddfc
@@ -9,7 +9,7 @@ import (
 	"errors"
 	"fmt"
 	"maps"
-	"sort"
+	"slices"
 	"strconv"
 	"strings"

@@ -56,7 +56,7 @@ func (a *application) environmentHosts(hosts map[string]settings.Host) (map[stri
 	if a.deps.Getenv("GITEA_TOKEN") == "" || (insecure == "" && sshHost == "") {
 		return hosts, nil
 	}
-	instance, err := settings.NormalizeHost(a.tokenInstance())
+	instance, err := settings.ResolveHost(a.tokenInstance(), hosts)
 	if err != nil {
 		return hosts, nil // The token's instance check reports this.
 	}
@@ -80,7 +80,36 @@ func (a *application) environmentHosts(hosts map[string]settings.Host) (map[stri
 	return result, nil
 }

+// targetRequest says what a command needs resolved.
+type targetRequest struct {
+	requireRepo, authenticate bool
+	// repo names the repository a command argument selected, such as the repository of an
+	// issue URL; it takes precedence over discovery and must agree with --repo.
+	repo, repoSource string
+}
+
 func (a *application) resolveTarget(ctx context.Context, requireRepo, authenticate bool) (target, error) {
+	return a.resolveRequest(ctx, targetRequest{requireRepo: requireRepo, authenticate: authenticate})
+}
+
+func (a *application) resolveRequest(ctx context.Context, req targetRequest) (target, error) {
+	t, err := a.connect(ctx, req)
+	if err != nil {
+		return t, err
+	}
+	// A bare repository NAME belongs to the account tea acts as, which is known only now.
+	if t.Repo != "" && !strings.Contains(t.Repo, "/") {
+		owner := cmp.Or(a.options.sudo, t.User)
+		if owner == "" {
+			return t, clierr.Wrap(clierr.Usage, fmt.Errorf("cannot tell who owns repository %q; use OWNER/%s", t.Repo, t.Repo))
+		}
+		t.Repo = owner + "/" + t.Repo
+	}
+	return t, nil
+}
+
+// connect selects the instance, account and repository and builds the authenticated client.
+func (a *application) connect(ctx context.Context, req targetRequest) (target, error) {
 	s, err := a.deps.Settings.Load()
 	if err != nil {
 		return target{}, err
@@ -89,142 +118,38 @@ func (a *application) resolveTarget(ctx context.Context, requireRepo, authentica
 		return target{}, err
 	}
 	t := target{}
-	rawRepo := a.options.repo
-	if rawRepo != "" {
-		t.RepoSource = "--repo"
-	} else if rawRepo = a.deps.Getenv("TEA_REPO"); rawRepo != "" {
-		t.RepoSource = "TEA_REPO"
-	}
-	if rawRepo == "" && requireRepo {
-		// Discovery probes fail outside a checkout; Git's diagnostics would only precede tea's own error.
-		probe := func(args ...string) (string, error) { return a.gitProbe().Output(ctx, a.options.cwd, false, args...) }
-		if saved, saveErr := probe("config", "--get", "tea.repository"); saveErr == nil && saved != "" {
-			rawRepo = saved
-			t.RepoSource = "git config tea.repository"
-		} else {
-			remote := a.options.remote
-			if remote == "" {
-				remote = s.Preferences.Remote
-			}
-			remote, err = git.DiscoverRemote(ctx, a.gitProbe(), a.options.cwd, remote)
-			if errors.Is(err, git.ErrAmbiguousRemote) {
-				return t, clierr.Wrap(clierr.Usage, err)
-			}
-			if err != nil {
-				return t, clierr.Wrap(clierr.Usage, errRepositoryRequired)
-			}
-			rawRepo, err = probe("remote", "get-url", remote)
-			if err != nil {
-				return t, fmt.Errorf("cannot read the URL of Git remote %q: %w", remote, err)
-			}
-			t.RepoSource = "remote " + remote
-		}
+	rawRepo, err := a.repositorySource(ctx, &t, s, req)
+	if err != nil {
+		return t, err
 	}
 	if rawRepo != "" {
-		if settings.ValidSlug(rawRepo) {
-			t.Repo = rawRepo
-		} else {
-			configured := s.Hosts.Hosts
-			if a.options.host != "" {
-				explicit, normalizeErr := settings.NormalizeHost(a.options.host)
-				if normalizeErr != nil {
-					return t, clierr.Wrap(clierr.Usage, normalizeErr)
-				}
-				if host, exists := configured[explicit]; exists {
-					configured = map[string]settings.Host{explicit: host}
-				}
-			}
-			t.Host, t.Repo, err = settings.RepositoryURL(rawRepo, configured)
-			if err != nil {
-				return t, clierr.Wrap(clierr.Usage, err)
-			}
-			t.HostSource = t.RepoSource
+		if err := a.selectRepository(ctx, &t, s, rawRepo); err != nil {
+			return t, err
 		}
 	}
-	if a.options.host != "" {
-		explicit, err := settings.NormalizeHost(a.options.host)
-		if err != nil {
-			return t, clierr.Wrap(clierr.Usage, err)
-		}
-		if t.Host != "" && t.Host != explicit {
-			return t, clierr.Wrap(clierr.Usage, errors.New("--host conflicts with repository instance"))
-		}
-		t.Host, t.HostSource = explicit, "--host"
-	}
-	if t.Host == "" {
-		for _, key := range []string{"TEA_HOST", "GITEA_INSTANCE_URL"} {
-			if value := a.deps.Getenv(key); value != "" {
-				t.Host, err = settings.NormalizeHost(value)
-				if err != nil {
-					return t, clierr.Wrap(clierr.Usage, err)
-				}
-				t.HostSource = key
-				break
-			}
-		}
-	}
-	if t.Host == "" && s.Preferences.DefaultHost != "" {
-		t.Host, t.HostSource = s.Preferences.DefaultHost, "config.yml default_host"
-	}
-	if t.Host == "" && len(s.Hosts.Hosts) == 1 {
-		for host := range s.Hosts.Hosts {
-			t.Host, t.HostSource = host, "sole configured instance"
-		}
-	}
-	if t.Host == "" {
-		return t, clierr.Wrap(clierr.Usage, errors.New("instance required: select --host or set default_host"))
+	if err := a.selectHost(&t, s); err != nil {
+		return t, err
 	}
 	host := s.Hosts.Hosts[t.Host]
 	t.host = host
 	t.gitProtocol = cmp.Or(host.GitProtocol, s.Preferences.GitProtocol)
-	t.User, t.UserSource = a.selectedUser()
-	if matched, ok := matchAccount(host.Users, t.User); ok {
-		t.User = matched
+	if err := a.selectAccount(ctx, &t, s, req.authenticate); err != nil {
+		return t, err
 	}
-	if t.User == "" {
-		t.User, t.UserSource = host.User, "active instance account"
-	}
-	if t.User == "" {
-		switch len(host.Users) {
-		case 1:
-			for user := range host.Users {
-				t.User, t.UserSource = user, "sole configured account"
-			}
-		case 0:
-		default:
-			if authenticate && a.deps.Getenv("GITEA_TOKEN") == "" && a.policy(s).CanPrompt(a.deps.Streams) && a.deps.Prompter != nil {
-				names := make([]string, 0, len(host.Users))
-				for user := range host.Users {
-					names = append(names, user)
-				}
-				sort.Strings(names)
-				selected, err := a.deps.Prompter.Input(ctx, "account ("+strings.Join(names, ", ")+")")
-				if err != nil {
-					return t, err
-				}
-				selected = strings.TrimSpace(selected)
-				matched, ok := matchAccount(host.Users, selected)
-				if !ok {
-					return t, clierr.Wrap(clierr.Usage, fmt.Errorf("account %q is not configured on %s", selected, t.Host))
-				}
-				t.User, t.UserSource = matched, "interactive account selection"
-			}
-		}
-	}
-	if !authenticate {
+	if !req.authenticate {
 		return t, nil
 	}
 	token := a.deps.Getenv("GITEA_TOKEN")
 	environmentCredential := token != ""
 	if token != "" {
-		normalized, err := settings.NormalizeHost(a.tokenInstance())
+		normalized, err := settings.ResolveHost(a.tokenInstance(), s.Hosts.Hosts)
 		if err != nil || normalized != t.Host {
 			return t, clierr.Wrap(clierr.Authentication, errors.New("GITEA_TOKEN requires GITEA_INSTANCE_URL or TEA_HOST matching the selected instance"))
 		}
 	} else {
 		account, exists := host.Users[t.User]
 		if !exists {
-			return t, clierr.Wrap(clierr.Authentication, fmt.Errorf("no selected account on %s; run tea auth login or tea auth switch", t.Host))
+			return t, clierr.Wrap(clierr.Authentication, t.accountError(t.User))
 		}
 		if account.Tokenless() {
 			signer, err := auth.SignerForAccount(account, a.sshPassphrase(ctx, a.policy(s), account.SSHKey))
@@ -272,6 +197,207 @@ func (a *application) resolveTarget(ctx context.Context, requireRepo, authentica
 	return t, nil
 }

+// repositorySource returns the raw repository the command targets and records its source: an
+// argument's repository, --repo, TEA_REPO, the saved tea.repository, or a discovered Git remote.
+func (a *application) repositorySource(ctx context.Context, t *target, s settings.Snapshot, req targetRequest) (string, error) {
+	if req.repo != "" {
+		if err := a.agreesWithRepoFlag(req, s.Hosts.Hosts); err != nil {
+			return "", err
+		}
+		t.RepoSource = req.repoSource
+		return req.repo, nil
+	}
+	if a.options.repo != "" {
+		t.RepoSource = "--repo"
+		return a.options.repo, nil
+	}
+	if raw := a.deps.Getenv("TEA_REPO"); raw != "" {
+		t.RepoSource = "TEA_REPO"
+		return raw, nil
+	}
+	if !req.requireRepo {
+		return "", nil
+	}
+	// Discovery probes fail outside a checkout; Git's diagnostics would only precede tea's own error.
+	probe := func(args ...string) (string, error) { return a.gitProbe().Output(ctx, a.options.cwd, false, args...) }
+	if saved, err := probe("config", "--get", "tea.repository"); err == nil && saved != "" {
+		t.RepoSource = "git config tea.repository"
+		return saved, nil
+	}
+	remote := cmp.Or(a.options.remote, s.Preferences.Remote)
+	remote, err := git.DiscoverRemote(ctx, a.gitProbe(), a.options.cwd, remote)
+	if errors.Is(err, git.ErrAmbiguousRemote) {
+		return "", clierr.Wrap(clierr.Usage, err)
+	}
+	if err != nil {
+		return "", clierr.Wrap(clierr.Usage, errRepositoryRequired)
+	}
+	raw, err := probe("remote", "get-url", remote)
+	if err != nil {
+		return "", fmt.Errorf("cannot read the URL of Git remote %q: %w", remote, err)
+	}
+	t.RepoSource = "remote " + remote
+	return raw, nil
+}
+
+// agreesWithRepoFlag rejects an argument naming another repository than --repo.
+func (a *application) agreesWithRepoFlag(req targetRequest, hosts map[string]settings.Host) error {
+	if a.options.repo == "" {
+		return nil
+	}
+	selected, selectedErr := settings.ParseRepository(req.repo, hosts)
+	explicit, explicitErr := settings.ParseRepository(a.options.repo, hosts)
+	if selectedErr != nil || explicitErr != nil {
+		return nil // Resolution reports the invalid one.
+	}
+	sameHost := selected.Host == "" || explicit.Host == "" || selected.Host == explicit.Host
+	sameRepo := strings.EqualFold(selected.Name, explicit.Name) && (explicit.Owner == "" || strings.EqualFold(selected.Owner, explicit.Owner))
+	if sameHost && sameRepo {
+		return nil
+	}
+	return clierr.Wrap(clierr.Usage, fmt.Errorf("%s points to %s but --repo is %s", req.repoSource, selected.Slug(), a.options.repo))
+}
+
+// selectRepository parses raw into the target's repository and, for URLs and HOST/OWNER/REPO,
+// its instance. With --host, URLs match only that instance.
+func (a *application) selectRepository(ctx context.Context, t *target, s settings.Snapshot, raw string) error {
+	configured := s.Hosts.Hosts
+	if a.options.host != "" {
+		explicit, err := settings.ResolveHost(a.options.host, configured)
+		if err != nil {
+			return clierr.Wrap(clierr.Usage, err)
+		}
+		if host, exists := configured[explicit]; exists {
+			configured = map[string]settings.Host{explicit: host}
+		}
+	}
+	ref, err := settings.ParseRepository(raw, configured)
+	if err != nil {
+		return clierr.Wrap(clierr.Usage, err)
+	}
+	if ref.Owner == "" && ref.Host == "" && a.isGitRemote(ctx, ref.Name) {
+		return clierr.Wrap(clierr.Usage, fmt.Errorf("invalid repository %q: use OWNER/REPO, HOST/OWNER/REPO or a URL; did you mean --remote %s?", raw, ref.Name))
+	}
+	t.Repo = ref.Slug()
+	if ref.Host != "" {
+		t.Host, t.HostSource = ref.Host, t.RepoSource
+	}
+	return nil
+}
+
+// isGitRemote reports whether name is a remote of the working directory's Git checkout.
+func (a *application) isGitRemote(ctx context.Context, name string) bool {
+	remotes, err := a.gitProbe().Output(ctx, a.options.cwd, false, "remote")
+	return err == nil && slices.Contains(strings.Fields(remotes), name)
+}
+
+// selectHost picks the instance: the repository's, --host, TEA_HOST or GITEA_INSTANCE_URL,
+// default_host, or the only configured instance.
+func (a *application) selectHost(t *target, s settings.Snapshot) error {
+	if a.options.host != "" {
+		explicit, err := settings.ResolveHost(a.options.host, s.Hosts.Hosts)
+		if err != nil {
+			return clierr.Wrap(clierr.Usage, err)
+		}
+		if t.Host != "" && t.Host != explicit {
+			return clierr.Wrap(clierr.Usage, fmt.Errorf("--host %s conflicts with %s from %s", explicit, t.Host, t.HostSource))
+		}
+		t.Host, t.HostSource = explicit, "--host"
+	}
+	if t.Host == "" {
+		for _, key := range []string{"TEA_HOST", "GITEA_INSTANCE_URL"} {
+			if value := a.deps.Getenv(key); value != "" {
+				host, err := settings.ResolveHost(value, s.Hosts.Hosts)
+				if err != nil {
+					return clierr.Wrap(clierr.Usage, fmt.Errorf("invalid %s: %w", key, err))
+				}
+				t.Host, t.HostSource = host, key
+				break
+			}
+		}
+	}
+	if t.Host == "" && s.Preferences.DefaultHost != "" {
+		t.Host, t.HostSource = s.Preferences.DefaultHost, "config.yml default_host"
+	}
+	if t.Host == "" && len(s.Hosts.Hosts) == 1 {
+		for host := range s.Hosts.Hosts {
+			t.Host, t.HostSource = host, "sole configured instance"
+		}
+	}
+	if t.Host == "" {
+		if len(s.Hosts.Hosts) == 0 {
+			return clierr.Wrap(clierr.Usage, errors.New("no instance selected: log in with tea auth login, or pass --host"))
+		}
+		return clierr.Wrap(clierr.Usage, fmt.Errorf("instance required: select --host or set default_host (instances: %s)", strings.Join(slices.Sorted(maps.Keys(s.Hosts.Hosts)), ", ")))
+	}
+	return nil
+}
+
+// selectAccount picks the account: --user or TEA_USER, the instance's active account, its only
+// account, or an interactive choice.
+func (a *application) selectAccount(ctx context.Context, t *target, s settings.Snapshot, authenticate bool) error {
+	users := t.host.Users
+	t.User, t.UserSource = a.selectedUser()
+	if matched, ok := matchAccount(users, t.User); ok {
+		t.User = matched
+	}
+	if t.User == "" {
+		t.User, t.UserSource = t.host.User, "active instance account"
+	}
+	if t.User != "" || len(users) == 0 {
+		return nil
+	}
+	if len(users) == 1 {
+		for user := range users {
+			t.User, t.UserSource = user, "sole configured account"
+		}
+		return nil
+	}
+	if !authenticate || a.deps.Getenv("GITEA_TOKEN") != "" || !a.policy(s).CanPrompt(a.deps.Streams) || a.deps.Prompter == nil {
+		return nil
+	}
+	selected, err := a.deps.Prompter.Input(ctx, "account ("+accountNames(users)+")")
+	if err != nil {
+		return err
+	}
+	matched, ok := matchAccount(users, strings.TrimSpace(selected))
+	if !ok {
+		return clierr.Wrap(clierr.Usage, t.accountError(strings.TrimSpace(selected)))
+	}
+	t.User, t.UserSource = matched, "interactive account selection"
+	return nil
+}
+
+// accountError explains why user has no stored credential on the target's instance.
+func (t target) accountError(user string) error {
+	users := t.host.Users
+	switch {
+	case user != "" && len(users) > 0:
+		return fmt.Errorf("account %q is not logged in on %s (accounts: %s)", user, t.Host, accountNames(users))
+	case user != "":
+		return fmt.Errorf("account %q is not logged in on %s; run tea auth login --host %s", user, t.Host, t.Host)
+	case len(users) > 0:
+		return fmt.Errorf("no active account on %s (accounts: %s); pass --user or run tea auth switch", t.Host, accountNames(users))
+	}
+	text := "no account for " + t.Host
+	if t.HostSource != "--host" {
+		text += " (instance from " + describeSource(t.HostSource) + ")"
+	}
+	return errors.New(text + "; run tea auth login --host " + t.Host + ", or use -R or --host to target another instance")
+}
+
+// describeSource words a target source for messages, such as git remote "origin".
+func describeSource(source string) string {
+	if remote, ok := strings.CutPrefix(source, "remote "); ok {
+		return "git remote " + strconv.Quote(remote)
+	}
+	return source
+}
+
+func accountNames(users map[string]settings.Account) string {
+	return strings.Join(slices.Sorted(maps.Keys(users)), ", ")
+}
+
 // selectedUser returns the account named by --user or TEA_USER, and which one named it.
 func (a *application) selectedUser() (string, string) {
 	if a.options.user != "" {
@@ -11,6 +11,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"strings"
 	"testing"

 	"gitea.dev/tea/internal/clierr"
@@ -232,6 +233,75 @@ func TestRepositoryResolutionPrecedence(t *testing.T) {
 	require.False(t, runner.prompt)
 }

+func TestRepositoryArgumentsAcceptShorthands(t *testing.T) {
+	server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		_, _ = w.Write([]byte(`{"full_name":"` + strings.TrimPrefix(r.URL.Path, "/api/v1/repos/") + `"}`))
+	}))
+	defer server.Close()
+	bare := strings.TrimPrefix(server.URL, "http://")
+	for _, tc := range []struct {
+		name string
+		args []string
+		want string
+	}{
+		{"bare name belongs to the active account", []string{"repo", "view", "project"}, "alice/project"},
+		{"bare name belongs to the sudo user", []string{"repo", "view", "project", "--sudo", "bob"}, "bob/project"},
+		{".git suffix", []string{"repo", "view", "bob/tool.git"}, "bob/tool"},
+		{"HOST/OWNER/REPO", []string{"repo", "view", bare + "/bob/tool"}, "bob/tool"},
+		{"--repo HOST/OWNER/REPO", []string{"repo", "view", "-R", bare + "/bob/tool.git"}, "bob/tool"},
+		{"bare --host", []string{"repo", "view", "bob/tool", "--host", bare}, "bob/tool"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			deps, out, _ := testDeps(t)
+			configureAccounts(t, deps, server.URL, "alice", "alice", "bob")
+			require.NoError(t, Execute(context.Background(), deps, append(tc.args, "--no-input", "--json=full_name")))
+			require.JSONEq(t, `{"full_name":"`+tc.want+`"}`, out.String())
+		})
+	}
+}
+
+func TestRepositoryNamedLikeAGitRemoteSuggestsRemote(t *testing.T) {
+	deps, _, _ := testDeps(t)
+	deps.Getenv = serverEnv("https://example.org", nil)
+	deps.Git = &fakeGit{results: map[string]string{"remote": "origin\nupstream"}}
+	err := Execute(context.Background(), deps, []string{"issue", "list", "-R", "origin"})
+	require.Equal(t, clierr.Usage, clierr.ExitCode(err))
+	require.EqualError(t, err, `invalid repository "origin": use OWNER/REPO, HOST/OWNER/REPO or a URL; did you mean --remote origin?`)
+	err = Execute(context.Background(), deps, []string{"issue", "list", "-R", "a/b/c/d"})
+	require.EqualError(t, err, `invalid repository "a/b/c/d": use OWNER/REPO, HOST/OWNER/REPO or a URL`)
+}
+
+func TestAccountErrorsNameTheInstanceAndAccounts(t *testing.T) {
+	const instance = "https://example.org"
+	remote := map[string]string{"remote": "origin", "remote get-url origin": "https://gitea.com/alice/project.git"}
+	for _, tc := range []struct {
+		name   string
+		users  []string
+		active string
+		args   []string
+		git    map[string]string
+		want   string
+	}{
+		{"unknown --user", []string{"alice", "bob"}, "alice", []string{"-R", "alice/project", "--user", "nobody"}, nil, `account "nobody" is not logged in on https://example.org (accounts: alice, bob)`},
+		{"unknown --user without accounts", nil, "", []string{"-R", "alice/project", "--user", "nobody", "--host", instance}, nil, `account "nobody" is not logged in on https://example.org; run tea auth login --host https://example.org`},
+		{"no active account", []string{"alice", "bob"}, "", []string{"-R", "alice/project"}, nil, "no active account on https://example.org (accounts: alice, bob); pass --user or run tea auth switch"},
+		{"instance from a remote", []string{"alice"}, "alice", nil, remote, `no account for https://gitea.com (instance from git remote "origin"); run tea auth login --host https://gitea.com, or use -R or --host to target another instance`},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			deps, _, _ := testDeps(t)
+			if tc.users != nil {
+				configureAccounts(t, deps, instance, tc.active, tc.users...)
+			}
+			if tc.git != nil {
+				deps.Git = &fakeGit{results: tc.git}
+			}
+			err := Execute(context.Background(), deps, append([]string{"issue", "list", "--no-input"}, tc.args...))
+			require.Equal(t, clierr.Authentication, clierr.ExitCode(err), "%v", err)
+			require.EqualError(t, err, tc.want)
+		})
+	}
+}
+
 func TestRepositoryDiscoveryOutsideCheckoutWritesNoGitDiagnostics(t *testing.T) {
 	if _, err := exec.LookPath("git"); err != nil {
 		t.Skip("Git is not installed")