diff --git a/internal/app/app.go b/internal/app/app.go index 55be3451..b283065a 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -521,7 +521,7 @@ func commandOutputFields(cmd *cobra.Command) []string { } case "tea": if cmd.Name() == "doctor" { - return jsonFields[target]() + return jsonFields[doctorReport]() } case "attachment": return jsonFields[services.IssueAttachment]() diff --git a/internal/app/app_test.go b/internal/app/app_test.go index baba80f0..0828fa31 100644 --- a/internal/app/app_test.go +++ b/internal/app/app_test.go @@ -427,7 +427,6 @@ const ( branch rename branch unprotect branch view - doctor issue attachment delete issue attachment download issue attachment edit diff --git a/internal/app/doctor.go b/internal/app/doctor.go index dcad7e74..d289cf65 100644 --- a/internal/app/doctor.go +++ b/internal/app/doctor.go @@ -4,27 +4,238 @@ package app import ( + "context" + "errors" "fmt" + "strings" + "time" "gitea.dev/tea/internal/clierr" + "gitea.dev/tea/internal/git" + "gitea.dev/tea/internal/services" + "gitea.dev/tea/internal/settings" + "gitea.dev/tea/internal/terminal" "github.com/spf13/cobra" ) -func (a *application) doctorCommand() *cobra.Command { - return &cobra.Command{Use: "doctor", Short: "Inspect resolved instance and repository context", Args: argsExact(0), RunE: func(cmd *cobra.Command, _ []string) error { - // Repository discovery is best-effort: doctor must also work outside a clone. - // Only discovery (usage) failures fall back; authentication errors are reported. - target, err := a.resolve(cmd.Context(), true) - if clierr.ExitCode(err) == clierr.Usage && a.options.repo == "" && a.deps.Getenv("TEA_REPO") == "" { - target, err = a.resolve(cmd.Context(), false) - } - if err != nil { - return err - } - if a.options.json != "" { - return a.writeJSON(cmd, target) - } - _, err = fmt.Fprintf(cmd.OutOrStdout(), "instance\t%s\t%s\naccount\t%s\t%s\nrepository\t%s\t%s\n", target.Host, target.HostSource, target.User, target.UserSource, target.Repo, target.RepoSource) - return err - }} +// Outcomes of a doctor check: only failed checks make tea doctor fail. +const ( + checkPassed = "ok" + checkWarning = "warning" + checkFailed = "failed" + checkSkipped = "skipped" +) + +// doctorCheck is one diagnosis of tea doctor, with the command that fixes a problem. +type doctorCheck struct { + Name string `json:"name"` + Status string `json:"status"` + Detail string `json:"detail"` + Fix string `json:"fix,omitempty"` +} + +// doctorReport is the instance, account and repository that commands select here, and the checks +// of each. +type doctorReport struct { + Host string `json:"host"` + User string `json:"user"` + Repo string `json:"repo"` + HostSource string `json:"host_source"` + UserSource string `json:"user_source"` + RepoSource string `json:"repo_source"` + Checks []doctorCheck `json:"checks"` +} + +// doctorTimeout bounds each request of a check. +const doctorTimeout = 10 * time.Second + +func (a *application) doctorCommand() *cobra.Command { + return &cobra.Command{ + Use: "doctor", Short: "Check tea's setup for this directory", Args: argsExact(0), + Long: `Check the setup that commands in this directory use: the settings files, the instance +and whether it answers, the account and its credential, the repository, and Git. Each +problem comes with the command that fixes it. tea doctor fails when a check fails.`, + Example: "$ tea doctor\n# Check another repository and instance\n$ tea doctor -R gitea.com/gitea/tea\n$ tea doctor --jq '.checks[] | select(.status == \"failed\")'", + RunE: func(cmd *cobra.Command, _ []string) error { + report := a.diagnose(cmd.Context()) + failed := 0 + for _, check := range report.Checks { + if check.Status == checkFailed { + failed++ + } + } + var err error + if a.options.json != "" { + err = a.writeJSON(cmd, report) + } else { + err = a.writeHuman(cmd, doctorTable(report.Checks, a.display())) + } + if err == nil && failed > 0 { + err = fmt.Errorf("%d of %d checks failed", failed, len(report.Checks)) + } + return err + }, + } +} + +// diagnose resolves the target like other commands, but reports each problem instead of stopping +// at the first. +func (a *application) diagnose(ctx context.Context) doctorReport { + var report doctorReport + add := func(check doctorCheck) { report.Checks = append(report.Checks, check) } + snapshot, err := a.deps.Settings.Load() + if err != nil { + add(doctorCheck{"Config", checkFailed, err.Error(), "repair or move the file; tea config path prints both files"}) + add(a.gitCheck(ctx, "")) + return report + } + add(doctorCheck{"Config", checkPassed, "config.yml and hosts.yml in " + a.deps.Settings.Dir, ""}) + t, err := a.resolveTarget(ctx, true, false) + repoErr := err + if err != nil && clierr.ExitCode(err) == clierr.Usage && a.options.repo == "" && a.deps.Getenv("TEA_REPO") == "" { + t, err = a.resolveTarget(ctx, false, false) + } + report.Host, report.User, report.Repo = t.Host, t.User, t.Repo + report.HostSource, report.UserSource, report.RepoSource = t.HostSource, t.UserSource, t.RepoSource + if t.Host == "" { + add(doctorCheck{"Instance", checkFailed, Explain(ctx, a.deps, err).Error(), "tea auth login --host URL, or pass --host"}) + add(a.gitCheck(ctx, "")) + return report + } + instance := a.instanceCheck(ctx, t) + add(instance) + authenticated, accountErr := a.resolveTarget(ctx, t.Repo != "", true) + switch { + case instance.Status == checkFailed: + accountErr = errors.New("instance does not answer") + detail := "not checked while the instance does not answer" + if t.User != "" { + detail = t.User + ": " + detail + } + add(doctorCheck{"Account", checkSkipped, detail, ""}) + case accountErr == nil: + report.User, report.UserSource = authenticated.User, authenticated.UserSource + fallthrough + default: + add(a.accountCheck(ctx, t, snapshot, authenticated, accountErr)) + } + switch { + case t.Repo == "": + detail := "none: not in a clone of a repository on " + t.Host + if repoErr != nil && !strings.HasPrefix(repoErr.Error(), errRepositoryRequired.Error()) { + detail = repoErr.Error() + } + add(doctorCheck{"Repository", checkSkipped, detail, ""}) + case accountErr != nil: + add(doctorCheck{"Repository", checkSkipped, t.Repo + " (" + describeSource(t.RepoSource) + "); not checked without a working account", ""}) + default: + add(a.repositoryCheck(ctx, authenticated)) + } + add(a.gitCheck(ctx, t.Host)) + if check, ok := sshCheck(t); ok { + add(check) + } + return report +} + +// instanceCheck asks the instance for its version. +func (a *application) instanceCheck(ctx context.Context, t target) doctorCheck { + ctx, cancel := context.WithTimeout(ctx, doctorTimeout) + defer cancel() + start := time.Now() + version, err := services.ServerVersion(ctx, a.deps.Client(t.Host, "", t.host.Insecure)) + where := t.Host + " (" + describeSource(t.HostSource) + ")" + if err != nil { + return doctorCheck{"Instance", checkFailed, where + ": " + Explain(ctx, a.deps, err).Error(), "check --host, the instance URL and your network"} + } + return doctorCheck{"Instance", checkPassed, fmt.Sprintf("%s · Gitea %s · %s", where, version, time.Since(start).Round(time.Millisecond)), ""} +} + +// accountCheck reports whether the account authenticates, and the scopes of its token. +func (a *application) accountCheck(ctx context.Context, t target, snapshot settings.Snapshot, authenticated target, err error) doctorCheck { + login := "tea auth login --host " + t.Host + if a.deps.Getenv("GITEA_TOKEN") != "" && err != nil { + instance, resolveErr := settings.ResolveHost(a.tokenInstance(), snapshot.Hosts.Hosts) + switch { + case a.tokenInstance() == "" || resolveErr != nil: + return doctorCheck{"Account", checkFailed, "GITEA_TOKEN is set, but neither GITEA_INSTANCE_URL nor TEA_HOST names its instance", "export GITEA_INSTANCE_URL=" + t.Host} + case instance != t.Host: + return doctorCheck{"Account", checkFailed, "GITEA_TOKEN belongs to " + instance + ", but the instance here is " + t.Host + " (" + describeSource(t.HostSource) + ")", "pass --host " + instance + " or -R a repository there, or unset GITEA_TOKEN"} + } + } + if err != nil { + detail := Explain(ctx, a.deps, err).Error() + if strings.Contains(detail, login) { + login = "" + } + return doctorCheck{"Account", checkFailed, detail, login} + } + ctx, cancel := context.WithTimeout(ctx, doctorTimeout) + defer cancel() + user, err := services.CurrentAccount(ctx, authenticated.client) + if err != nil { + return doctorCheck{"Account", checkFailed, authenticated.User + ": " + Explain(ctx, a.deps, err).Error(), login} + } + detail := user + " (" + describeSource(authenticated.UserSource) + ") · credential valid" + if scopes, err := services.TokenScopes(ctx, authenticated.client); err == nil && len(scopes) > 0 { + detail += " · scopes: " + strings.Join(scopes, ", ") + } + return doctorCheck{"Account", checkPassed, detail, ""} +} + +// repositoryCheck reports whether the account can read the repository. +func (a *application) repositoryCheck(ctx context.Context, t target) doctorCheck { + ctx, cancel := context.WithTimeout(ctx, doctorTimeout) + defer cancel() + where := t.Repo + " (" + describeSource(t.RepoSource) + ")" + repo, err := services.Repositories{Client: t.client}.View(ctx, t.Repo) + if err != nil { + return doctorCheck{"Repository", checkFailed, where + ": " + Explain(ctx, a.deps, err).Error(), "check the remote URL, or pass -R OWNER/REPO"} + } + return doctorCheck{"Repository", checkPassed, where + " · default branch " + repo.DefaultBranch, ""} +} + +// gitCheck reports the installed Git and whether it asks tea for the credentials of instance. +func (a *application) gitCheck(ctx context.Context, instance string) doctorCheck { + version, err := a.gitProbe().Output(ctx, a.options.cwd, false, "--version") + switch { + case err != nil: + return doctorCheck{"Git", checkFailed, "Git is not installed or cannot run", "install Git"} + case instance == "": + return doctorCheck{"Git", checkPassed, version, ""} + } + if scope := git.HelperScope(ctx, a.gitProbe(), a.options.cwd, instance); scope != "" { + return doctorCheck{"Git", checkPassed, version + " · tea is the credential helper (" + scope + ")", ""} + } + return doctorCheck{"Git", checkWarning, version + " · credential helper not configured for " + instance, "tea auth setup-git, if pushes over HTTPS should use this account"} +} + +// sshCheck reports where SSH remotes connect when the instance uses SSH. +func sshCheck(t target) (doctorCheck, bool) { + switch { + case t.host.SSHHost != "": + return doctorCheck{"SSH", checkPassed, "remotes connect to " + t.host.SSHHost + " (ssh_host)", ""}, true + case t.gitProtocol == "ssh": + return doctorCheck{"SSH", checkWarning, "remotes connect to the web host on port 22", "tea config set --host " + t.Host + " ssh_host HOST:PORT, if SSH listens elsewhere"}, true + } + return doctorCheck{}, false +} + +// doctorTable lists the checks: on a terminal with an icon per outcome and the fix under a problem, +// otherwise as tab-separated status, name, detail and fix. +func doctorTable(checks []doctorCheck, d terminal.Display) string { + icons := map[string]terminal.Cell{ + checkPassed: {Text: "✓", Color: "green"}, + checkWarning: {Text: "!", Color: "yellow"}, + checkFailed: {Text: "X", Color: "red"}, + checkSkipped: {Text: "-", Muted: true}, + } + rows := make([][]terminal.Cell, 0, len(checks)) + for _, check := range checks { + rows = append(rows, []terminal.Cell{icons[check.Status], {Text: check.Status}, {Text: check.Name, Bold: true}, {Text: check.Detail}, {Text: check.Fix}}) + if d.TTY && check.Fix != "" && check.Status != checkPassed { + rows = append(rows, []terminal.Cell{{}, {}, {}, {Text: "→ " + check.Fix, Muted: true}}) + } + } + return terminal.Table([]terminal.Column{{TTYOnly: true}, {PipedOnly: true}, {}, {}, {PipedOnly: true}}, rows, d) } diff --git a/internal/app/doctor_test.go b/internal/app/doctor_test.go index c1418109..5dd81714 100644 --- a/internal/app/doctor_test.go +++ b/internal/app/doctor_test.go @@ -5,17 +5,44 @@ package app import ( "context" + "encoding/json" "net/http" + "strings" "testing" + "gitea.dev/tea/internal/auth" "gitea.dev/tea/internal/clierr" + "gitea.dev/tea/internal/settings" "github.com/stretchr/testify/require" ) +// doctorServer is an instance that answers the requests of tea doctor for alice/project. +func doctorServer(t *testing.T) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v1/version": + _, _ = w.Write([]byte(`{"version":"28.1.0"}`)) + case "/api/v1/token": + _, _ = w.Write([]byte(`{"scopes":["all"]}`)) + case "/api/v1/repos/alice/project": + _, _ = w.Write([]byte(`{"full_name":"alice/project","default_branch":"main"}`)) + default: + t.Errorf("unexpected request %s", r.URL.Path) + w.WriteHeader(http.StatusNotFound) + } + }) +} + +// doctorGit is a Git checkout of alice/project at url, without tea as credential helper. +func doctorGit(url string) *fakeGit { + return &fakeGit{results: map[string]string{"--version": "git version 2.50.1", "remote": "origin", "remote get-url origin": url + "/alice/project.git"}} +} + func TestEnvironmentCredentialReportsActualAccount(t *testing.T) { - server := newAuthenticatedServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { t.Error("unexpected request") })) + server := newAuthenticatedServer(doctorServer(t)) defer server.Close() deps, out, _ := testDeps(t) + deps.Git = &fakeGit{results: map[string]string{"--version": "git version 2.50.1"}} deps.Getenv = serverEnv(server.URL, map[string]string{"TEA_USER": "ALICE"}) require.NoError(t, Execute(context.Background(), deps, []string{"doctor", "--json"})) require.Contains(t, out.String(), `"user":"alice"`) @@ -23,13 +50,90 @@ func TestEnvironmentCredentialReportsActualAccount(t *testing.T) { } func TestDoctorDiscoversRepositoryBestEffort(t *testing.T) { - server := newAuthenticatedServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { t.Error("unexpected request") })) + server := newAuthenticatedServer(doctorServer(t)) defer server.Close() deps, out, _ := serverDeps(t, server.URL) - deps.Git = &fakeGit{results: map[string]string{"remote": "origin", "remote get-url origin": server.URL + "/alice/project.git"}} + deps.Git = doctorGit(server.URL) require.NoError(t, Execute(context.Background(), deps, []string{"doctor", "--json=repo,repo_source"})) require.JSONEq(t, `{"repo":"alice/project","repo_source":"remote origin"}`, out.String()) - out.Reset() - deps.Getenv = envFunc(map[string]string{"TEA_HOST": server.URL}) - require.Equal(t, clierr.Authentication, clierr.ExitCode(Execute(context.Background(), deps, []string{"doctor"}))) +} + +func TestDoctorReportsEveryCheckInsteadOfFailingFirst(t *testing.T) { + server := newAuthenticatedServer(doctorServer(t)) + defer server.Close() + origin := `(git remote "origin")` + for _, tc := range []struct { + name string + env map[string]string + args []string + statuses string + account string + }{ + { + name: "environment token", env: map[string]string{"TEA_HOST": server.URL, "GITEA_TOKEN": "token"}, + statuses: "Config ok, Instance ok, Account ok, Repository ok, Git warning", + account: "alice (GITEA_TOKEN authenticated account) · credential valid · scopes: all", + }, + { + name: "no account for the instance of the remote", env: map[string]string{"TEA_HOST": server.URL}, + statuses: "Config ok, Instance ok, Account failed, Repository skipped, Git warning", + account: "no account for " + server.URL + " (instance from " + strings.Trim(origin, "()") + "); run tea auth login --host " + server.URL + ", or use -R or --host to target another instance", + }, + { + name: "token of another instance", env: map[string]string{"TEA_HOST": "https://other.example.org", "GITEA_TOKEN": "token"}, + statuses: "Config ok, Instance ok, Account failed, Repository skipped, Git warning", + account: "GITEA_TOKEN belongs to https://other.example.org, but the instance here is " + server.URL + " " + origin, + }, + { + name: "instance that does not answer", args: []string{"--host", "http://127.0.0.1:1", "-R", "alice/project"}, + statuses: "Config ok, Instance failed, Account skipped, Repository skipped, Git warning", + account: "not checked while the instance does not answer", + }, + } { + t.Run(tc.name, func(t *testing.T) { + deps, out, _ := testDeps(t) + deps.Git = doctorGit(server.URL) + deps.Getenv = envFunc(tc.env) + err := Execute(context.Background(), deps, append([]string{"doctor", "--json=checks"}, tc.args...)) + var report doctorReport + require.NoError(t, json.Unmarshal(out.Bytes(), &report), out.String()) + statuses := make([]string, len(report.Checks)) + for i, check := range report.Checks { + statuses[i] = check.Name + " " + check.Status + if check.Name == "Account" { + require.Equal(t, tc.account, check.Detail) + } + } + require.Equal(t, tc.statuses, strings.Join(statuses, ", ")) + if strings.Contains(tc.statuses, "failed") { + require.Equal(t, clierr.Failure, clierr.ExitCode(err)) + require.EqualError(t, err, "1 of 5 checks failed") + } else { + require.NoError(t, err) + } + }) + } +} + +func TestDoctorShowsChecksOnATerminal(t *testing.T) { + server := newAuthenticatedServer(doctorServer(t)) + defer server.Close() + deps, out, _ := testDeps(t) + deps.Streams.OutTTY = true + deps.Git = doctorGit(server.URL) + configureHost(t, deps, server.URL, settings.Host{User: "alice", Users: tokenAccounts(settings.StoreFile, "alice"), SSHHost: "ssh.example.org:2222"}) + require.NoError(t, deps.Credentials.Save(server.URL, "alice", settings.StoreFile, auth.Credential{AccessToken: "alice-token"})) + require.NoError(t, Execute(context.Background(), deps, []string{"doctor"})) + lines := withoutANSI(out.String()) + for _, want := range []string{ + "✓ Config config.yml and hosts.yml in " + deps.Settings.Dir + "\n", + "✓ Account alice (active instance account) · credential valid · scopes: all\n", + "✓ Repository alice/project (git remote \"origin\") · default branch main\n", + "! Git git version 2.50.1 · credential helper not configured for " + server.URL + "\n" + + " → tea auth setup-git, if pushes over HTTPS should use this account\n", + "✓ SSH remotes connect to ssh.example.org:2222 (ssh_host)\n", + } { + require.Contains(t, lines, want) + } + require.Regexp(t, `✓ Instance `+server.URL+` \(git remote "origin"\) · Gitea 28.1.0 · \d+ms\n`, lines) }