feat(branch): branch from tags and commits, protect with explicit rules

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 <bircni@icloud.com>
2026-10-26 19:11:25 +01:00 · 2026-10-11 23:52:02 +02:00
1 parent 682c9f66a2
commit 02cf01093f
@@ -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
 }
@@ -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)
 }