From 02cf01093fb7cae008782de8dedc6e562a1b5b45 Mon Sep 17 00:00:00 2001 From: bircni Date: Sun, 11 Oct 2026 23:40:13 +0200 Subject: [PATCH] feat(branch): branch from tags and commits, protect with explicit rules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit branch create --source v1.0.0 (or a commit) failed with "The old branch does not exist" because only old_branch_name was sent; the request now sends old_ref_name, through a raw call until the SDK has the field. branch protect nope exited 0 and created a rule for a missing branch, and protecting twice failed with "permission denied". Branches must now exist (--pattern protects a glob instead), an existing rule warns "! main is already protected", and unprotecting an unprotected branch warns instead of failing. The rule is explained in help and set with --required-approvals, --allow-push, --status-check, --block-on-rejected-reviews and --dismiss-stale-approvals. branch list shows NAME (marking the default branch), COMMIT, PROTECTED, UPDATED and MESSAGE, branch view is a full view with --web, branch delete takes several branches, and every change reports a ✓ line. Signed-off-by: bircni --- internal/app/app_test.go | 8 - internal/app/branches.go | 444 +++++++++++++++++++++++------ internal/app/branches_test.go | 75 ++++- internal/app/human_output.go | 8 + internal/app/human_output_test.go | 4 +- internal/app/organizations.go | 7 - internal/services/branches.go | 94 ++++-- internal/services/branches_test.go | 46 ++- 8 files changed, 555 insertions(+), 131 deletions(-) diff --git a/internal/app/app_test.go b/internal/app/app_test.go index 97f199e9..5d28e38b 100644 --- a/internal/app/app_test.go +++ b/internal/app/app_test.go @@ -419,13 +419,6 @@ const ( pr review cancel-request ` examplesToWrite = ` - branch create - branch delete - branch list - branch protect - branch rename - branch unprotect - branch view issue attachment delete issue attachment download issue attachment edit @@ -520,7 +513,6 @@ const ( wiki view ` longsToWrite = ` - branch issue issue attachment issue comment diff --git a/internal/app/branches.go b/internal/app/branches.go index 0795a813..4fc86c24 100644 --- a/internal/app/branches.go +++ b/internal/app/branches.go @@ -6,6 +6,7 @@ package app import ( "errors" "fmt" + "net/url" "strconv" "strings" @@ -33,110 +34,375 @@ func validateBranch(name string) error { return nil } +// validateBranches validates every branch name. +func validateBranches(names []string) error { + for _, name := range names { + if err := validateBranch(name); err != nil { + return err + } + } + return nil +} + func (a *application) branchCommand() *cobra.Command { - group := &cobra.Command{Use: "branch", Short: "Manage remote branches and protection rules"} + group := &cobra.Command{ + Use: "branch", + Short: "Manage remote branches and protection rules", + Long: `Work with the branches of a repository on the server: list and view them, create, rename and +delete them, and protect them from direct pushes. To work with local branches, use git.`, + } + group.AddCommand(a.branchListCommand(), a.branchViewCommand(), a.branchCreateCommand(), a.branchRenameCommand(), a.branchDeleteCommand()) + group.AddCommand(a.branchProtectCommand(), a.branchUnprotectCommand()) + return group +} + +// branchURL links a branch's files on the web. +func branchURL(repository services.Repository, branch string) string { + if repository.URL == "" { + return "" + } + return repository.URL + "/src/branch/" + strings.ReplaceAll(url.PathEscape(branch), "%2F", "/") +} + +func (a *application) branchListCommand() *cobra.Command { var limit, page int - list := &cobra.Command{Use: "list", Short: "List remote branches", Args: argsExact(0), RunE: func(cmd *cobra.Command, _ []string) error { - target, err := a.resolve(cmd.Context(), true) - if err != nil { - return err - } - items, err := (services.Branches{Client: target.client}).List(cmd.Context(), target.Repo, limit, page) - if err != nil { - return err - } - return writeList(a, cmd, items, branchRow) - }} - pageFlags(list, &limit, &page) - group.AddCommand(list) - for _, verb := range []string{"view", "create", "rename", "delete", "protect", "unprotect"} { - var source string - count := 1 - use := verb + " " - if verb == "rename" { - count = 2 - use = "rename " - } - arguments := argsExact(count) - if verb == "protect" || verb == "unprotect" { - use = verb + " ..." - arguments = usageArgs(cobra.MinimumNArgs(1)) - } - command := &cobra.Command{Use: use, Short: titleCase(verb) + " a remote branch", Args: arguments, RunE: func(cmd *cobra.Command, args []string) error { - for _, name := range args { - if err := validateBranch(name); err != nil { + command := &cobra.Command{ + Use: "list", + Short: "List branches", + Example: `$ tea branch list +$ tea branch list -R alice/project --limit 100 --json name,sha`, + Args: argsExact(0), + RunE: func(cmd *cobra.Command, _ []string) error { + target, err := a.resolve(cmd.Context(), true) + if err != nil { + return err + } + repository := services.Repository{} + if a.options.json == "" { + if repository, err = (services.Repositories{Client: target.client}).View(cmd.Context(), target.Repo); err != nil { return err } } - if source != "" { - if err := validateBranch(source); err != nil { - return err - } + result, err := (services.Branches{Client: target.client}).ListPage(cmd.Context(), target.Repo, limit, page) + if err != nil { + return err } - if verb == "rename" && args[0] == args[1] { + spec := listSpec[services.Branch]{ + Columns: []terminal.Column{{Header: "Name"}, {Header: "Commit"}, {Header: "Protected"}, {Header: "Updated"}, {Header: "Message", Flex: true}}, + Row: func(item services.Branch) []terminal.Cell { + name := terminal.Cell{Text: item.Name, Link: branchURL(repository, item.Name)} + if item.Name == repository.DefaultBranch { + name.Text += " (default)" + name.Bold = true + } + protected := "" + if item.Protected { + protected = "protected" + } + message, _, _ := strings.Cut(item.Message, "\n") + return []terminal.Cell{name, {Text: shortSHA(item.SHA), Muted: true}, {Text: protected, Color: "yellow"}, {Time: terminal.ParseTime(item.UpdatedAt)}, {Text: message}} + }, + Context: "branches in " + target.Repo, Empty: "no branches in " + target.Repo, + } + return writeTable(a, cmd, result.Items, spec, listPage{Number: page, More: result.More, Total: result.Total, Counted: result.Counted}) + }, + } + pageFlags(command, &limit, &page) + return command +} + +func (a *application) branchViewCommand() *cobra.Command { + var web bool + command := &cobra.Command{ + Use: "view ", + Short: "View a branch", + Example: "$ tea branch view main\n$ tea branch view feature/login --web", + Args: argsExact(1), + RunE: func(cmd *cobra.Command, args []string) error { + if err := validateBranch(args[0]); err != nil { + return err + } + target, err := a.resolve(cmd.Context(), true) + if err != nil { + return err + } + repository, err := (services.Repositories{Client: target.client}).View(cmd.Context(), target.Repo) + if err != nil { + return err + } + if web { + return a.openBrowser(cmd, branchURL(repository, args[0])) + } + item, err := (services.Branches{Client: target.client}).View(cmd.Context(), target.Repo, args[0]) + if err != nil { + return err + } + return writeView(a, cmd, item, func(item services.Branch) terminal.View { + var badges []terminal.Cell + if item.Name == repository.DefaultBranch { + badges = append(badges, terminal.Cell{Text: "default", Color: "green"}) + } + protection := "" + if item.Protected { + badges = append(badges, terminal.Cell{Text: "protected", Color: "yellow"}) + protection = countNoun(int(item.RequiredApprovals), "required approval") + } + meta := "" + if updated := terminal.ParseTime(item.UpdatedAt); !updated.IsZero() { + meta = "updated " + terminal.RelativeTime(a.deps.Now(), updated) + } + return terminal.View{ + Title: item.Name, Badges: badges, Meta: meta, Body: item.Message, Kind: "branch", URL: branchURL(repository, item.Name), + Fields: []terminal.Field{{Label: "Commit", Value: item.SHA}, {Label: "Protection", Value: protection}, {Label: "You can push", Value: yesNo(item.CanPush)}}, + } + }) + }, + } + command.Flags().BoolVarP(&web, "web", "w", false, "Open the branch in the browser") + command.ValidArgsFunction = a.completeBranches() + return command +} + +func (a *application) branchCreateCommand() *cobra.Command { + var source string + command := &cobra.Command{ + Use: "create ", + Short: "Create a branch", + Long: "Create a branch on the server from another branch, a tag or a commit; the default branch unless --source names one.", + Example: `# Branch off the default branch +$ tea branch create feature/login + +# Branch off a tag or a commit +$ tea branch create hotfix/1.0.1 --source v1.0.0 +$ tea branch create bisect --source 3f2a9c1`, + Args: argsExact(1), + RunE: func(cmd *cobra.Command, args []string) error { + if err := validateBranch(args[0]); err != nil { + return err + } + if cmd.Flags().Changed("source") && strings.TrimSpace(source) == "" { + return clierr.Wrap(clierr.Usage, errors.New("--source cannot be empty")) + } + target, err := a.resolve(cmd.Context(), true) + if err != nil { + return err + } + item, err := (services.Branches{Client: target.client}).Create(cmd.Context(), target.Repo, args[0], source) + if err != nil { + return err + } + if source == "" { + a.done(cmd, "Created branch %s in %s at %s", item.Name, target.Repo, shortSHA(item.SHA)) + } else { + a.done(cmd, "Created branch %s from %s in %s at %s", item.Name, source, target.Repo, shortSHA(item.SHA)) + } + return a.writeMutationJSON(cmd, item) + }, + } + command.Flags().StringVarP(&source, "source", "s", "", "Branch, tag or commit to start from (default: the default branch)") + _ = command.RegisterFlagCompletionFunc("source", mergeCompletions(a.completeBranches(), a.completeTags())) + return command +} + +func (a *application) branchRenameCommand() *cobra.Command { + command := &cobra.Command{ + Use: "rename ", + Short: "Rename a branch", + Example: "$ tea branch rename feature/logn feature/login", + Annotations: textOutput("branch rename does not return a resource"), + Args: argsExact(2), + RunE: func(cmd *cobra.Command, args []string) error { + if err := validateBranches(args); err != nil { + return err + } + if args[0] == args[1] { return clierr.Wrap(clierr.Usage, errors.New("new branch name must differ from the current name")) } target, err := a.resolve(cmd.Context(), true) if err != nil { return err } - service := services.Branches{Client: target.client} - if verb == "delete" || verb == "unprotect" { - if err := a.confirm(cmd, verb+" branches "+strings.Join(args, ", ")+" in "+target.Repo); err != nil { - return err - } - } - switch verb { - case "view": - item, err := service.View(cmd.Context(), target.Repo, args[0]) - if err != nil { - return err - } - return writeItem(a, cmd, item, branchRow) - case "create": - item, err := service.Create(cmd.Context(), target.Repo, args[0], source) - if err != nil { - return err - } - return writeItem(a, cmd, item, branchRow) - case "rename": - return service.Rename(cmd.Context(), target.Repo, args[0], args[1]) - case "delete": - return service.Delete(cmd.Context(), target.Repo, args[0]) - case "protect", "unprotect": - var failures []error - for _, name := range args { - var err error - if verb == "protect" { - err = service.Protect(cmd.Context(), target.Repo, name) - } else { - err = service.Unprotect(cmd.Context(), target.Repo, name) - } - if err != nil { - failures = append(failures, fmt.Errorf("%s branch %s: %w", verb, name, err)) - continue - } - if len(args) > 1 { - if _, err := fmt.Fprintf(cmd.ErrOrStderr(), "%s branch %s\n", verb, name); err != nil { - return err - } - } - } - return errors.Join(failures...) + if err := (services.Branches{Client: target.client}).Rename(cmd.Context(), target.Repo, args[0], args[1]); err != nil { + return err } + a.done(cmd, "Renamed branch %s to %s in %s", args[0], args[1], target.Repo) return nil - }} - if verb == "create" { - command.Flags().StringVar(&source, "source", "", "Source branch (default: repository default branch)") - } - if verb != "view" && verb != "create" { - command.Annotations = textOutput("branch " + verb + " does not return a resource") - } - group.AddCommand(command) + }, } - return group + command.ValidArgsFunction = a.completeBranches() + return command } -func branchRow(item services.Branch) []terminal.Cell { - return terminal.Row(item.Name, item.SHA, strconv.FormatBool(item.Protected)) +func (a *application) branchDeleteCommand() *cobra.Command { + command := &cobra.Command{ + Use: "delete ...", + Short: "Delete branches", + Example: `$ tea branch delete feature/login +$ tea branch delete old-a old-b --yes`, + Annotations: map[string]string{"tea.confirm": "true", "tea.text": "branch deletion does not return a resource"}, + Args: usageArgs(cobra.MinimumNArgs(1)), + RunE: func(cmd *cobra.Command, args []string) error { + if err := validateBranches(args); err != nil { + return err + } + target, err := a.resolve(cmd.Context(), true) + if err != nil { + return err + } + subject := branchSubject(args) + if err := a.confirmAction(cmd, terminal.Confirmation{Prompt: "Delete " + subject + " from " + target.Repo + "?", NonTTY: "delete " + subject + " from " + target.Repo}); err != nil { + return err + } + service := services.Branches{Client: target.client} + return batchDone(a, cmd, args, func(name string) string { return "delete branch " + name }, func(name string) (string, error) { + return name, service.Delete(cmd.Context(), target.Repo, name) + }, func(name string) string { return "Deleted branch " + name + " from " + target.Repo }) + }, + } + command.ValidArgsFunction = a.completeBranches() + return command +} + +// branchSubject names one or more branches in a prompt: "branch main" or "2 branches (a, b)". +func branchSubject(names []string) string { + if len(names) == 1 { + return "branch " + names[0] + } + return fmt.Sprintf("%d branches (%s)", len(names), strings.Join(names, ", ")) +} + +func (a *application) branchProtectCommand() *cobra.Command { + var rule services.Protection + var pattern string + var pushers []string + command := &cobra.Command{ + Use: "protect ...", + Short: "Protect branches", + Long: `Add a protection rule for each branch. By default nobody can push to a protected branch directly +and pull requests need one approving review to merge; --allow-push lets users or teams push, and +--required-approvals changes the review count. + +The branches must exist. --pattern protects every branch that matches a glob instead, such as +release/*, including branches created later.`, + Example: `# Require two approvals and passing CI on main +$ tea branch protect main --required-approvals 2 --status-check ci/build + +# Let the release team push to every release branch +$ tea branch protect --pattern 'release/*' --allow-push acme/release`, + Annotations: textOutput("branch protect does not return a resource"), + Args: func(cmd *cobra.Command, args []string) error { + if pattern != "" { + return usageArgs(cobra.NoArgs)(cmd, args) + } + return usageArgs(cobra.MinimumNArgs(1))(cmd, args) + }, + RunE: func(cmd *cobra.Command, args []string) error { + if err := validateBranches(args); err != nil { + return err + } + if rule.RequiredApprovals < 0 { + return clierr.Wrap(clierr.Usage, errors.New("--required-approvals cannot be negative")) + } + if err := requireNames("push", "", pushers, rule.StatusChecks); err != nil { + return err + } + for _, pusher := range pushers { + if _, team, ok := strings.Cut(pusher, "/"); ok { + rule.PushTeams = append(rule.PushTeams, team) + } else { + rule.PushUsers = append(rule.PushUsers, pusher) + } + } + target, err := a.resolve(cmd.Context(), true) + if err != nil { + return err + } + service := services.Branches{Client: target.client} + rules := args + if pattern != "" { + rules = []string{pattern} + } + _, failures := applyBatch(cmd, rules, func(name string) string { return "protect " + name }, func(name string) (string, error) { + if pattern == "" { + if _, err := service.View(cmd.Context(), target.Repo, name); err != nil { + return "", services.Classify(err, "branch", strconv.Quote(name), target.Repo) + } + } + protection := rule + protection.Rule = name + created, err := service.Protect(cmd.Context(), target.Repo, protection) + if err == nil && !created { + a.warn(cmd, "%s is already protected in %s; change its rule on the web", name, target.Repo) + return "", nil + } + return name, err + }, func(name string) { + if name != "" { + a.done(cmd, "Protected %s in %s (%s)", name, target.Repo, protectionSummary(rule)) + } + }) + return errors.Join(failures...) + }, + } + flags := command.Flags() + flags.StringVar(&pattern, "pattern", "", "Protect the branches matching this glob, such as release/*") + flags.Int64Var(&rule.RequiredApprovals, "required-approvals", 1, "Approving reviews that pull requests need") + flags.StringSliceVar(&pushers, "allow-push", nil, "Users, or teams as ORG/TEAM, that may push directly") + flags.StringSliceVar(&rule.StatusChecks, "status-check", nil, "Status checks that must pass (repeatable)") + flags.BoolVar(&rule.BlockOnRejected, "block-on-rejected-reviews", false, "Block merging while changes are requested") + flags.BoolVar(&rule.DismissStaleApprovals, "dismiss-stale-approvals", false, "Dismiss approvals when new commits are pushed") + _ = command.RegisterFlagCompletionFunc("allow-push", a.completeUsers()) + command.ValidArgsFunction = a.completeBranches() + return command +} + +// protectionSummary describes a protection rule, as in "1 approval required, no direct pushes". +func protectionSummary(rule services.Protection) string { + parts := []string{countNoun(int(rule.RequiredApprovals), "approval") + " required"} + if pushers := len(rule.PushUsers) + len(rule.PushTeams); pushers > 0 { + parts = append(parts, "pushes by "+strings.Join(append(append([]string{}, rule.PushUsers...), rule.PushTeams...), ", ")) + } else { + parts = append(parts, "no direct pushes") + } + if len(rule.StatusChecks) > 0 { + parts = append(parts, "checks "+strings.Join(rule.StatusChecks, ", ")) + } + return strings.Join(parts, ", ") +} + +func (a *application) branchUnprotectCommand() *cobra.Command { + command := &cobra.Command{ + Use: "unprotect ...", + Short: "Remove branch protection", + Long: "Remove the protection rules of branches, or of glob patterns such as release/*.", + Example: "$ tea branch unprotect main\n$ tea branch unprotect 'release/*' --yes", + Annotations: map[string]string{"tea.confirm": "true", "tea.text": "branch unprotect does not return a resource"}, + Args: usageArgs(cobra.MinimumNArgs(1)), + RunE: func(cmd *cobra.Command, args []string) error { + target, err := a.resolve(cmd.Context(), true) + if err != nil { + return err + } + subject := "the protection of " + strings.Join(args, ", ") + if err := a.confirmAction(cmd, terminal.Confirmation{Prompt: "Remove " + subject + " in " + target.Repo + "?", NonTTY: "remove " + subject + " in " + target.Repo}); err != nil { + return err + } + service := services.Branches{Client: target.client} + _, failures := applyBatch(cmd, args, func(name string) string { return "unprotect " + name }, func(name string) (string, error) { + removed, err := service.Unprotect(cmd.Context(), target.Repo, name) + if err == nil && !removed { + a.warn(cmd, "%s is not protected in %s", name, target.Repo) + return "", nil + } + return name, err + }, func(name string) { + if name != "" { + a.done(cmd, "Unprotected %s in %s", name, target.Repo) + } + }) + return errors.Join(failures...) + }, + } + command.ValidArgsFunction = a.completeBranches() + return command } diff --git a/internal/app/branches_test.go b/internal/app/branches_test.go index 44883f9e..fd45132a 100644 --- a/internal/app/branches_test.go +++ b/internal/app/branches_test.go @@ -5,6 +5,7 @@ package app import ( "context" + "encoding/json" "net/http" "strings" "testing" @@ -33,13 +34,81 @@ func TestBulkBranchProtectionContinuesAfterFailure(t *testing.T) { w.WriteHeader(http.StatusNoContent) })) defer server.Close() - deps, out, diagnostics := serverDeps(t, server.URL) + deps, out, diagnostics := repoTTYDeps(t, server.URL) err := Execute(context.Background(), deps, []string{"branch", "unprotect", "failed", "succeeded", "-R", "owner/repo", "--no-input"}) require.Error(t, err) require.Empty(t, names) err = Execute(context.Background(), deps, []string{"branch", "unprotect", "failed", "succeeded", "-R", "owner/repo", "--yes"}) - require.ErrorContains(t, err, "failed") + require.ErrorContains(t, err, "unprotect failed") require.Equal(t, []string{"failed", "succeeded"}, names) require.Empty(t, out.String()) - require.Contains(t, diagnostics.String(), "succeeded") + require.Equal(t, "✓ Unprotected succeeded in owner/repo\n", diagnostics.String()) +} + +func TestBranchProtectChecksTheBranchAndIsIdempotent(t *testing.T) { + var requests []string + server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests = append(requests, r.Method+" "+r.URL.Path) + switch r.Method + " " + r.URL.Path { + case "GET /api/v1/repos/alice/x/branches/main", "GET /api/v1/repos/alice/x/branches/dev": + _, _ = w.Write([]byte(`{"name":"main"}`)) + case "POST /api/v1/repos/alice/x/branch_protections": + var rule struct { + Name string `json:"rule_name"` + } + require.NoError(t, json.NewDecoder(r.Body).Decode(&rule)) + if rule.Name == "main" { + w.WriteHeader(http.StatusForbidden) + _, _ = w.Write([]byte(`{"message":"Branch protection already exist"}`)) + return + } + _, _ = w.Write([]byte(`{"rule_name":"dev"}`)) + case "DELETE /api/v1/repos/alice/x/branch_protections/dev": + w.WriteHeader(http.StatusNotFound) + case "GET /api/v1/repos/alice/x": + _, _ = w.Write([]byte(`{"full_name":"alice/x"}`)) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer server.Close() + deps, _, diagnostics := repoTTYDeps(t, server.URL) + err := Execute(context.Background(), deps, []string{"branch", "protect", "nope", "-R", "alice/x"}) + require.EqualError(t, err, `protect nope: branch "nope" not found in alice/x`) + require.Equal(t, []string{"GET /api/v1/repos/alice/x/branches/nope"}, requests) + require.NoError(t, Execute(context.Background(), deps, []string{"branch", "protect", "main", "dev", "-R", "alice/x"})) + require.Equal(t, "! main is already protected in alice/x; change its rule on the web\n"+ + "✓ Protected dev in alice/x (1 approval required, no direct pushes)\n", diagnostics.String()) + diagnostics.Reset() + require.NoError(t, Execute(context.Background(), deps, []string{"branch", "unprotect", "dev", "-R", "alice/x", "--yes"})) + require.Equal(t, "! dev is not protected in alice/x\n", diagnostics.String()) +} + +func TestBranchCreateFromATag(t *testing.T) { + server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requirePayload(t, `{"new_branch_name":"hotfix","old_ref_name":"v1.0.0"}`, r.Body) + _, _ = w.Write([]byte(`{"name":"hotfix","commit":{"id":"abcdef123"}}`)) + })) + defer server.Close() + deps, out, diagnostics := repoTTYDeps(t, server.URL) + require.NoError(t, Execute(context.Background(), deps, []string{"branch", "create", "hotfix", "--source", "v1.0.0", "-R", "alice/x"})) + require.Empty(t, out.String()) + require.Equal(t, "✓ Created branch hotfix from v1.0.0 in alice/x at abcdef1\n", diagnostics.String()) +} + +func TestBranchDeleteTakesSeveralBranches(t *testing.T) { + var deleted []string + server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, http.MethodDelete, r.Method) + deleted = append(deleted, strings.TrimPrefix(r.URL.Path, "/api/v1/repos/alice/x/branches/")) + w.WriteHeader(http.StatusNoContent) + })) + defer server.Close() + deps, _, diagnostics := repoTTYDeps(t, server.URL) + prompt := &scriptedPrompter{accept: true} + deps.Prompter, deps.Streams.InTTY, deps.Streams.ErrTTY = prompt, true, true + require.NoError(t, Execute(context.Background(), deps, []string{"branch", "rm", "old-a", "old-b", "-R", "alice/x"})) + require.Equal(t, []string{"Delete 2 branches (old-a, old-b) from alice/x?"}, prompt.confirmations) + require.Equal(t, []string{"old-a", "old-b"}, deleted) + require.Equal(t, "✓ Deleted branch old-a from alice/x\n✓ Deleted branch old-b from alice/x\n", diagnostics.String()) } diff --git a/internal/app/human_output.go b/internal/app/human_output.go index 6b4014c1..40556062 100644 --- a/internal/app/human_output.go +++ b/internal/app/human_output.go @@ -340,3 +340,11 @@ func (a *application) morePagesHint(cmd *cobra.Command, more bool, page int) err _, err := fmt.Fprintf(cmd.ErrOrStderr(), "more results available; use --page %d\n", page+1) return err } + +// yesNo renders a boolean cell. +func yesNo(value bool) string { + if value { + return "yes" + } + return "no" +} diff --git a/internal/app/human_output_test.go b/internal/app/human_output_test.go index 854d7d6f..a1d2c539 100644 --- a/internal/app/human_output_test.go +++ b/internal/app/human_output_test.go @@ -157,6 +157,8 @@ func TestMultiItemHumanOutputPagesOnce(t *testing.T) { _, _ = w.Write([]byte("[" + issue(1) + "," + issue(2) + "]")) case "/api/v1/repos/alice/project/issues/1", "/api/v1/repos/alice/project/issues/2": _, _ = w.Write([]byte(issue(int(r.URL.Path[len(r.URL.Path)-1] - '0')))) + case "/api/v1/repos/alice/project": + _, _ = w.Write([]byte(`{"full_name":"alice/project","default_branch":"main"}`)) case "/api/v1/repos/alice/project/branches": _, _ = w.Write([]byte(`[{"name":"main","commit":{"id":"abc"}},{"name":"dev","commit":{"id":"def"}}]`)) case "/api/v1/repos/alice/project/issues/5/comments": @@ -172,7 +174,7 @@ func TestMultiItemHumanOutputPagesOnce(t *testing.T) { }{ {[]string{"milestone", "issue", "list", "3"}, "1 Issue 1 closed\n2 Issue 2 closed\n"}, {[]string{"issue", "close", "1", "2"}, "1 Issue 1 closed\n2 Issue 2 closed\n"}, - {[]string{"branch", "list"}, "main abc false\ndev def false\n"}, + {[]string{"branch", "list"}, "Showing 2 of 2 branches in alice/project\n\nNAME COMMIT PROTECTED UPDATED MESSAGE\nmain (default) abc\ndev def\n"}, {[]string{"issue", "comment", "list", "5"}, "1 alice\none\n\n2 bob\ntwo\n\n"}, } { t.Run(tc.args[0]+" "+tc.args[1], func(t *testing.T) { diff --git a/internal/app/organizations.go b/internal/app/organizations.go index c7edebc7..30f9f994 100644 --- a/internal/app/organizations.go +++ b/internal/app/organizations.go @@ -52,13 +52,6 @@ func timeField(label, raw string, d terminal.Display) terminal.Field { return terminal.Field{Label: label, Value: terminal.RelativeTime(now, t)} } -func yesNo(value bool) string { - if value { - return "yes" - } - return "no" -} - // countOf words a count, such as "1 member" or "2 members". func countOf(n int, singular, plural string) string { if n == 1 { diff --git a/internal/services/branches.go b/internal/services/branches.go index af181511..6961c31b 100644 --- a/internal/services/branches.go +++ b/internal/services/branches.go @@ -5,15 +5,20 @@ package services import ( "context" + "errors" + "net/http" + "strings" gitea "gitea.dev/sdk" "gitea.dev/tea/internal/transport" ) -// Branch describes a remote branch and its protection. +// Branch describes a remote branch, its last commit and its protection. type Branch struct { Name string `json:"name"` SHA string `json:"sha"` + Message string `json:"message"` + UpdatedAt string `json:"updated_at"` Protected bool `json:"protected"` RequiredApprovals int64 `json:"required_approvals"` CanPush bool `json:"can_push"` @@ -26,7 +31,7 @@ func branchFrom(branch *gitea.Branch) Branch { CanPush: branch.UserCanPush, CanMerge: branch.UserCanMerge, } if branch.Commit != nil { - result.SHA = branch.Commit.ID + result.SHA, result.Message, result.UpdatedAt = branch.Commit.ID, branch.Commit.Message, timestamp(branch.Commit.Timestamp) } return result } @@ -36,13 +41,25 @@ type Branches struct{ Client *transport.Client } // List returns a page of remote branches. func (s Branches) List(ctx context.Context, repo string, limit, page int) ([]Branch, error) { - branches, err := repoCall(ctx, s.Client, repo, func(ctx context.Context, c *gitea.Client, owner, name string) ([]*gitea.Branch, *gitea.Response, error) { - return c.Repositories.ListRepoBranches(ctx, owner, name, gitea.ListRepoBranchesOptions{ListOptions: listOptions(limit, page)}) + window, err := s.ListPage(ctx, repo, limit, page) + return window.Items, err +} + +// ListPage is List that also reports whether more branches follow and how many there are. +func (s Branches) ListPage(ctx context.Context, repo string, limit, page int) (Page[Branch], error) { + window, err := listPage(limit, page, func(limit, page int) ([]*gitea.Branch, http.Header, error) { + var headers http.Header + branches, err := repoCall(ctx, s.Client, repo, func(ctx context.Context, c *gitea.Client, owner, name string) ([]*gitea.Branch, *gitea.Response, error) { + branches, resp, err := c.Repositories.ListRepoBranches(ctx, owner, name, gitea.ListRepoBranchesOptions{ListOptions: listOptions(limit, page)}) + headers = headerOf(resp) + return branches, resp, err + }) + return branches, headers, err }) if err != nil { - return nil, err + return Page[Branch]{}, err } - return convertAll(branches, branchFrom), nil + return Page[Branch]{Items: convertAll(window.Items, branchFrom), More: window.More, Total: window.Total, Counted: window.Counted}, nil } // View retrieves a remote branch. @@ -52,11 +69,23 @@ func (s Branches) View(ctx context.Context, repo, branch string) (Branch, error) }) } -// Create creates a branch from the supplied source or repository default. +// Create creates a branch from source, a branch, tag or commit, or from the default branch. func (s Branches) Create(ctx context.Context, repo, branch, source string) (Branch, error) { - return repoConvert(ctx, s.Client, repo, branchFrom, func(ctx context.Context, c *gitea.Client, owner, name string) (*gitea.Branch, *gitea.Response, error) { - return c.Repositories.CreateBranch(ctx, owner, name, gitea.CreateBranchOption{BranchName: branch, OldBranchName: source}) - }) + if source == "" { + return repoConvert(ctx, s.Client, repo, branchFrom, func(ctx context.Context, c *gitea.Client, owner, name string) (*gitea.Branch, *gitea.Response, error) { + return c.Repositories.CreateBranch(ctx, owner, name, gitea.CreateBranchOption{BranchName: branch}) + }) + } + if _, _, err := splitRepository(repo); err != nil { + return Branch{}, err + } + // TODO(sdk): CreateBranchOption lacks old_ref_name, which also accepts tags and commits. + var created gitea.Branch + payload := map[string]string{"new_branch_name": branch, "old_ref_name": source} + if err := sendJSON(ctx, s.Client, http.MethodPost, "repos/"+repo+"/branches", payload, &created); err != nil { + return Branch{}, err + } + return branchFrom(&created), nil } // Rename renames a remote branch. @@ -75,19 +104,48 @@ func (s Branches) Delete(ctx context.Context, repo, branch string) error { }) } -// Protect blocks direct pushes to the branch and requires one approving review. -func (s Branches) Protect(ctx context.Context, repo, branch string) error { - return repoDo(ctx, s.Client, repo, func(ctx context.Context, c *gitea.Client, owner, name string) (*gitea.Response, error) { +// Protection is a branch protection rule. +type Protection struct { + // Rule is the branch name, or a glob pattern such as release/*, that the rule protects. + Rule string `json:"rule"` + RequiredApprovals int64 `json:"required_approvals"` + // PushUsers and PushTeams may push directly; without them nobody can. + PushUsers []string `json:"push_users"` + PushTeams []string `json:"push_teams"` + // StatusChecks must pass before pull requests merge. + StatusChecks []string `json:"status_checks"` + BlockOnRejected bool `json:"block_on_rejected_reviews"` + DismissStaleApprovals bool `json:"dismiss_stale_approvals"` +} + +// Protect adds a protection rule. It reports false without a change when the rule exists already. +func (s Branches) Protect(ctx context.Context, repo string, rule Protection) (bool, error) { + push := len(rule.PushUsers)+len(rule.PushTeams) > 0 + err := repoDo(ctx, s.Client, repo, func(ctx context.Context, c *gitea.Client, owner, name string) (*gitea.Response, error) { _, resp, err := c.Repositories.CreateBranchProtection(ctx, owner, name, gitea.CreateBranchProtectionOption{ - BranchName: branch, RuleName: branch, RequiredApprovals: 1, + RuleName: rule.Rule, RequiredApprovals: rule.RequiredApprovals, + EnablePush: push, EnablePushWhitelist: push, PushWhitelistUsernames: rule.PushUsers, PushWhitelistTeams: rule.PushTeams, + EnableStatusCheck: len(rule.StatusChecks) > 0, StatusCheckContexts: rule.StatusChecks, + BlockOnRejectedReviews: rule.BlockOnRejected, DismissStaleApprovals: rule.DismissStaleApprovals, }) return resp, err }) + if apiErr, ok := errors.AsType[*transport.APIError](err); ok && strings.Contains(strings.ToLower(apiErr.Message), "already exist") { + return false, nil + } + return err == nil, err } -// Unprotect removes the named branch protection rule. -func (s Branches) Unprotect(ctx context.Context, repo, branch string) error { - return repoDo(ctx, s.Client, repo, func(ctx context.Context, c *gitea.Client, owner, name string) (*gitea.Response, error) { - return c.Repositories.DeleteBranchProtection(ctx, owner, name, branch) +// Unprotect removes the protection rule named rule. It reports false when there is none. +func (s Branches) Unprotect(ctx context.Context, repo, rule string) (bool, error) { + err := repoDo(ctx, s.Client, repo, func(ctx context.Context, c *gitea.Client, owner, name string) (*gitea.Response, error) { + return c.Repositories.DeleteBranchProtection(ctx, owner, name, rule) }) + if apiErr, ok := errors.AsType[*transport.APIError](err); ok && apiErr.Status == http.StatusNotFound { + if _, err := (Repositories{Client: s.Client}).get(ctx, repo); err != nil { + return false, err + } + return false, nil + } + return err == nil, err } diff --git a/internal/services/branches_test.go b/internal/services/branches_test.go index 663f4138..997b0627 100644 --- a/internal/services/branches_test.go +++ b/internal/services/branches_test.go @@ -34,12 +34,12 @@ func TestBranchListMapsServerFields(t *testing.T) { assert.Equal(t, "/api/v1/repos/owner/repo/branches", r.URL.Path) assert.Equal(t, "20", r.URL.Query().Get("limit")) assert.Equal(t, "3", r.URL.Query().Get("page")) - _, _ = w.Write([]byte(`[{"name":"main","commit":{"id":"abc"},"protected":true,"required_approvals":2,"user_can_push":true,"user_can_merge":true},{"name":"orphan","commit":null}]`)) + _, _ = w.Write([]byte(`[{"name":"main","commit":{"id":"abc","message":"Fix\n\nbody","timestamp":"2026-10-01T12:00:00Z"},"protected":true,"required_approvals":2,"user_can_push":true,"user_can_merge":true},{"name":"orphan","commit":null}]`)) })) items, err := (Branches{Client: client}).List(context.Background(), "owner/repo", 20, 3) require.NoError(t, err) require.Equal(t, []Branch{ - {Name: "main", SHA: "abc", Protected: true, RequiredApprovals: 2, CanPush: true, CanMerge: true}, + {Name: "main", SHA: "abc", Message: "Fix\n\nbody", UpdatedAt: "2026-10-01T12:00:00Z", Protected: true, RequiredApprovals: 2, CanPush: true, CanMerge: true}, {Name: "orphan"}, }, items) } @@ -50,11 +50,10 @@ func TestBranchCreateAndProtectPayloads(t *testing.T) { switch r.URL.Path { case "/api/v1/repos/owner/repo/branches": assert.Equal(t, "feature", payload["new_branch_name"]) - assert.Equal(t, "", payload["old_branch_name"]) + assert.Empty(t, payload["old_branch_name"]) w.WriteHeader(http.StatusCreated) _, _ = w.Write([]byte(`{"name":"feature","commit":{"id":"abc"}}`)) case "/api/v1/repos/owner/repo/branch_protections": - assert.Equal(t, "main", payload["branch_name"]) assert.Equal(t, "main", payload["rule_name"]) assert.Equal(t, false, payload["enable_push"]) assert.EqualValues(t, 1, payload["required_approvals"]) @@ -68,5 +67,42 @@ func TestBranchCreateAndProtectPayloads(t *testing.T) { item, err := service.Create(context.Background(), "owner/repo", "feature", "") require.NoError(t, err) require.Equal(t, Branch{Name: "feature", SHA: "abc"}, item) - require.NoError(t, service.Protect(context.Background(), "owner/repo", "main")) + created, err := service.Protect(context.Background(), "owner/repo", Protection{Rule: "main", RequiredApprovals: 1}) + require.NoError(t, err) + require.True(t, created) +} + +func TestBranchCreateFromATagOrCommitSendsOldRefName(t *testing.T) { + client := newTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, http.MethodPost+" /api/v1/repos/owner/repo/branches", r.Method+" "+r.URL.Path) + assert.Equal(t, map[string]any{"new_branch_name": "hotfix", "old_ref_name": "v1.0.0"}, requestJSON(t, r)) + w.WriteHeader(http.StatusCreated) + _, _ = w.Write([]byte(`{"name":"hotfix","commit":{"id":"abc"}}`)) + })) + item, err := (Branches{Client: client}).Create(context.Background(), "owner/repo", "hotfix", "v1.0.0") + require.NoError(t, err) + require.Equal(t, Branch{Name: "hotfix", SHA: "abc"}, item) +} + +func TestBranchProtectionIsIdempotent(t *testing.T) { + client := newTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.Method + " " + r.URL.Path { + case "POST /api/v1/repos/owner/repo/branch_protections": + w.WriteHeader(http.StatusForbidden) + _, _ = w.Write([]byte(`{"message":"Branch protection already exist"}`)) + case "DELETE /api/v1/repos/owner/repo/branch_protections/main": + w.WriteHeader(http.StatusNotFound) + case "GET /api/v1/repos/owner/repo": + _, _ = w.Write([]byte(`{"full_name":"owner/repo"}`)) + default: + t.Errorf("unexpected request %s %s", r.Method, r.URL.Path) + } + })) + service := Branches{Client: client} + created, err := service.Protect(context.Background(), "owner/repo", Protection{Rule: "main"}) + require.NoError(t, err) + require.False(t, created) + removed, err := service.Unprotect(context.Background(), "owner/repo", "main") + require.NoError(t, err) + require.False(t, removed) }