fix: send a team's permission before its units (Gitea 28), hidden private-org team reviews, long non-ASCII repo descriptions, -R NAME vs directory guard, flake fallback

Signed-off-by: bircni <bircni@icloud.com>
This commit is contained in:
bircni committed 2026-10-18 21:30:21 +02:00
1 parent 33f3ff10b6
commit 757b5fa1b9
12 files changed
+150 -8

No files matched your search

+3
View File
@@ -384,6 +384,9 @@ archive, or run `make docs`.
`--editor`, not as a third argument. `repo create` names its template options `--template` `--editor`, not as a third argument. `repo create` names its template options `--template`
and `--trust-model`; see `tea repo create --help`. and `--trust-model`; see `tea repo create --help`.
- `--repo` takes `OWNER/REPO` or a URL; use `--cwd DIR` to point at a local checkout. - `--repo` takes `OWNER/REPO` or a URL; use `--cwd DIR` to point at a local checkout.
A bare name (`-R project`) means a repository of your account, so it is refused when a
directory with that name exists in the working directory: write `-R OWNER/project`, or
`--cwd project` for the checkout.
- `pr checkout N` (alias `co`) checks out a local branch named after the PR's - `pr checkout N` (alias `co`) checks out a local branch named after the PR's
head branch when that name is free or already points at the head; otherwise head branch when that name is free or already points at the head; otherwise
it uses `OWNER/BRANCH` for a fork or `pr/N`. The branch tracks the head it uses `OWNER/BRANCH` for a fork or `pr/N`. The branch tracks the head
+2 -1
View File
@@ -15,7 +15,8 @@
devShells.default = pkgs.mkShell { devShells.default = pkgs.mkShell {
name = "tea-dev-environment"; name = "tea-dev-environment";
buildInputs = with pkgs; [ buildInputs = with pkgs; [
go_1_27 # flake.lock may pin a nixpkgs that predates Go 1.27; go.mod's toolchain line then fetches it.
(pkgs.go_1_27 or pkgs.go)
gopls gopls
gnumake gnumake
# Add other dependencies here if needed # Add other dependencies here if needed
+3 -2
View File
@@ -11,6 +11,7 @@ import (
"path/filepath" "path/filepath"
"slices" "slices"
"strings" "strings"
"unicode/utf8"
"gitea.dev/tea/internal/clierr" "gitea.dev/tea/internal/clierr"
"gitea.dev/tea/internal/services" "gitea.dev/tea/internal/services"
@@ -209,8 +210,8 @@ func validateCreateFlags(cmd *cobra.Command, opts services.RepositoryOptions, or
return clierr.Wrap(clierr.Usage, errors.New("--team requires --org and a team name")) return clierr.Wrap(clierr.Usage, errors.New("--team requires --org and a team name"))
case changed("template") && template == "": case changed("template") && template == "":
return clierr.Wrap(clierr.Usage, errors.New("--template requires owner/repo or a repository URL")) return clierr.Wrap(clierr.Usage, errors.New("--template requires owner/repo or a repository URL"))
case len(opts.Description) > 2048: case utf8.RuneCountInString(opts.Description) > 2048:
return clierr.Wrap(clierr.Usage, errors.New("description cannot exceed 2048 bytes")) return clierr.Wrap(clierr.Usage, errors.New("description cannot exceed 2048 characters"))
case template == "" && (opts.Gitignores != "" || opts.License != "" || opts.Readme != "" || opts.DefaultBranch != "") && !opts.AutoInit: case template == "" && (opts.Gitignores != "" || opts.License != "" || opts.Readme != "" || opts.DefaultBranch != "") && !opts.AutoInit:
return clierr.Wrap(clierr.Usage, errors.New("initial content and default branch options require --auto-init")) return clierr.Wrap(clierr.Usage, errors.New("initial content and default branch options require --auto-init"))
case template != "" && changed("auto-init", "label-template", "gitignores", "license", "readme", "as-template", "trust-model", "object-format"): case template != "" && changed("auto-init", "label-template", "gitignores", "license", "readme", "as-template", "trust-model", "object-format"):
+6
View File
@@ -9,6 +9,8 @@ import (
"errors" "errors"
"fmt" "fmt"
"maps" "maps"
"os"
"path/filepath"
"slices" "slices"
"strconv" "strconv"
"strings" "strings"
@@ -102,6 +104,10 @@ func (a *application) resolveRequest(ctx context.Context, req targetRequest) (ta
} }
// A bare repository NAME belongs to the account tea acts as, which is known only now. // A bare repository NAME belongs to the account tea acts as, which is known only now.
if t.Repo != "" && !strings.Contains(t.Repo, "/") { if t.Repo != "" && !strings.Contains(t.Repo, "/") {
// A name that is also a directory here may be meant as that checkout, as in tea -R ./project.
if info, statErr := os.Stat(filepath.Join(cmp.Or(a.options.cwd, "."), t.Repo)); statErr == nil && info.IsDir() && t.RepoSource == "--repo" {
return t, clierr.Wrap(clierr.Usage, fmt.Errorf("%q is both a repository name and a directory here; use OWNER/%s for the repository, or --cwd %s for that checkout", t.Repo, t.Repo, t.Repo))
}
owner := cmp.Or(a.options.sudo, t.User) owner := cmp.Or(a.options.sudo, t.User)
if owner == "" { if owner == "" {
return t, clierr.Wrap(clierr.Usage, fmt.Errorf("cannot tell who owns repository %q; use OWNER/%s", t.Repo, t.Repo)) return t, clierr.Wrap(clierr.Usage, fmt.Errorf("cannot tell who owns repository %q; use OWNER/%s", t.Repo, t.Repo))
+18
View File
@@ -376,3 +376,21 @@ func TestEnvironmentCredentialOverridesTheActiveAccount(t *testing.T) {
require.NoError(t, Execute(context.Background(), deps, []string{"issue", "create", "-R", "alice/project", "--title", "Issue", "--body", "Body", "--no-input", "--json"})) require.NoError(t, Execute(context.Background(), deps, []string{"issue", "create", "-R", "alice/project", "--title", "Issue", "--body", "Body", "--no-input", "--json"}))
require.True(t, created) require.True(t, created)
} }
func TestRepositoryFlagNamedLikeADirectoryIsAmbiguous(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()
dir := t.TempDir()
require.NoError(t, os.Mkdir(filepath.Join(dir, "project"), 0o755))
deps, out, _ := testDeps(t)
configureAccounts(t, deps, server.URL, "alice", "alice")
err := Execute(context.Background(), deps, []string{"repo", "view", "-R", "project", "--cwd", dir, "--no-input", "--json=full_name"})
require.Equal(t, clierr.Usage, clierr.ExitCode(err))
require.ErrorContains(t, err, `"project" is both a repository name and a directory here; use OWNER/project`)
// A qualified name, or a bare name without such a directory, is unambiguous.
require.NoError(t, Execute(context.Background(), deps, []string{"repo", "view", "-R", "alice/project", "--cwd", dir, "--no-input", "--json=full_name"}))
require.NoError(t, Execute(context.Background(), deps, []string{"repo", "view", "-R", "other", "--cwd", dir, "--no-input", "--json=full_name"}))
require.Contains(t, out.String(), "alice/other")
}
+4 -2
View File
@@ -216,12 +216,14 @@ func TestTeamPermissionSetsEveryUnit(t *testing.T) {
} }
require.NoError(t, Execute(context.Background(), deps, []string{"team", "create", "acme/devs", "--permission", "write"})) require.NoError(t, Execute(context.Background(), deps, []string{"team", "create", "acme/devs", "--permission", "write"}))
require.NoError(t, Execute(context.Background(), deps, []string{"team", "edit", "5", "--permission", "read", "--unit", "repo.wiki=write"})) require.NoError(t, Execute(context.Background(), deps, []string{"team", "edit", "5", "--permission", "read", "--unit", "repo.wiki=write"}))
require.Len(t, sent, 2) require.Len(t, sent, 3)
require.Equal(t, "devs", sent[0]["name"]) require.Equal(t, "devs", sent[0]["name"])
require.Equal(t, every("write"), sent[0]["units_map"]) require.Equal(t, every("write"), sent[0]["units_map"])
// The permission goes first on its own, because Gitea 28 keeps an admin team admin otherwise.
require.Equal(t, map[string]any{"name": "devs", "permission": "read"}, sent[1])
wikiWrite := every("read") wikiWrite := every("read")
wikiWrite["repo.wiki"] = "write" wikiWrite["repo.wiki"] = "write"
require.Equal(t, map[string]any{"permission": "read", "units_map": wikiWrite}, sent[1]) require.Equal(t, map[string]any{"permission": "read", "units_map": wikiWrite}, sent[2])
} }
func TestTeamMemberRemoveConfirmsOnce(t *testing.T) { func TestTeamMemberRemoveConfirmsOnce(t *testing.T) {
+20 -1
View File
@@ -6,6 +6,7 @@ package services
import ( import (
"context" "context"
"errors" "errors"
"fmt"
"net/http" "net/http"
"slices" "slices"
"strconv" "strconv"
@@ -238,6 +239,13 @@ func (s Repositories) Create(ctx context.Context, org string, opts RepositoryOpt
Readme: opts.Readme, DefaultBranch: opts.DefaultBranch, TrustModel: gitea.TrustModel(opts.TrustModel), Readme: opts.Readme, DefaultBranch: opts.DefaultBranch, TrustModel: gitea.TrustModel(opts.TrustModel),
ObjectFormatName: opts.ObjectFormat, ObjectFormatName: opts.ObjectFormat,
} }
// TODO(sdk): CreateRepoOption.Validate counts the description in bytes, but Gitea allows 2048
// characters; a longer one is set by an edit once the repository exists.
description := opts.Description
later := len(description) > sdkDescriptionLimit
if later {
options.Description = ""
}
item, _, err := call(ctx, s.Client, func(ctx context.Context, c *gitea.Client) (*gitea.Repository, *gitea.Response, error) { item, _, err := call(ctx, s.Client, func(ctx context.Context, c *gitea.Client) (*gitea.Repository, *gitea.Response, error) {
if org != "" { if org != "" {
return c.Repositories.CreateOrgRepo(ctx, org, options) return c.Repositories.CreateOrgRepo(ctx, org, options)
@@ -251,9 +259,20 @@ func (s Repositories) Create(ctx context.Context, org string, opts RepositoryOpt
} }
return Repository{}, Classify(err, "repository", "", "") return Repository{}, Classify(err, "repository", "", "")
} }
return repositoryFrom(item), nil created := repositoryFrom(item)
if later {
edited, err := s.Edit(ctx, created.FullName, RepositoryEdit{Description: &description})
if err != nil {
return created, fmt.Errorf("repository %s was created, but setting its description failed: %w", created.FullName, err)
}
return edited, nil
}
return created, nil
} }
// sdkDescriptionLimit is the length in bytes that the SDK accepts for a new repository's description.
const sdkDescriptionLimit = 2048
// Edit updates only explicitly supplied resource fields. // Edit updates only explicitly supplied resource fields.
func (s Repositories) Edit(ctx context.Context, repo string, opts RepositoryEdit) (Repository, error) { func (s Repositories) Edit(ctx context.Context, repo string, opts RepositoryEdit) (Repository, error) {
var mergeStyle *gitea.MergeStyle var mergeStyle *gitea.MergeStyle
+22
View File
@@ -6,6 +6,7 @@ package services
import ( import (
"context" "context"
"net/http" "net/http"
"strings"
"testing" "testing"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
@@ -118,3 +119,24 @@ func TestRepositoryReadme(t *testing.T) {
_, _, err = (Repositories{Client: client}).Readme(context.Background(), "alice/project", "develp") _, _, err = (Repositories{Client: client}).Readme(context.Background(), "alice/project", "develp")
require.EqualError(t, err, `branch "develp" not found in alice/project`) require.EqualError(t, err, `branch "develp" not found in alice/project`)
} }
func TestRepositoryCreateSetsADescriptionBeyondTheSDKByteLimitByEdit(t *testing.T) {
description := strings.Repeat("日", 800) // 2400 bytes, 800 characters: Gitea accepts it, the SDK would not
var methods []string
client := newTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
methods = append(methods, r.Method)
body := requestJSON(t, r)
switch r.Method {
case http.MethodPost:
require.Empty(t, body["description"])
_, _ = w.Write([]byte(`{"id":1,"name":"project","full_name":"alice/project"}`))
case http.MethodPatch:
require.Equal(t, description, body["description"])
_, _ = w.Write([]byte(`{"id":1,"name":"project","full_name":"alice/project","description":"` + description + `"}`))
}
}))
created, err := (Repositories{Client: client}).Create(context.Background(), "", RepositoryOptions{Name: "project", Description: description})
require.NoError(t, err)
require.Equal(t, []string{http.MethodPost, http.MethodPatch}, methods)
require.Equal(t, description, created.Description)
}
+14 -2
View File
@@ -99,7 +99,8 @@ func (s Reviews) changeRequests(ctx context.Context, n int64, changes ReviewRequ
// CheckRequestTeams verifies that every named team exists in the repository's organization. Gitea // CheckRequestTeams verifies that every named team exists in the repository's organization. Gitea
// lets only organization members list teams, though outside collaborators and the pull request // lets only organization members list teams, though outside collaborators and the pull request
// author may request reviews; when the lookup is forbidden the server checks the names, as for gh. // author may request reviews; when the lookup is forbidden, or the organization is hidden from the
// caller (a private organization answers 404 to non-members), the server checks the names, as for gh.
func (s Reviews) CheckRequestTeams(ctx context.Context, names []string) error { func (s Reviews) CheckRequestTeams(ctx context.Context, names []string) error {
owner, _, _ := strings.Cut(s.Repository, "/") owner, _, _ := strings.Cut(s.Repository, "/")
_, isOrg, err := organization(ctx, s.Client, owner) _, isOrg, err := organization(ctx, s.Client, owner)
@@ -110,10 +111,21 @@ func (s Reviews) CheckRequestTeams(ctx context.Context, names []string) error {
return err return err
} }
if !isOrg { if !isOrg {
// A private organization answers 404 to non-members, like a name nobody owns; a user that
// exists is certainly not an organization.
_, _, err := call(ctx, s.Client, func(ctx context.Context, c *gitea.Client) (*gitea.User, *gitea.Response, error) {
return c.Users.GetUserInfo(ctx, owner)
})
if IsNotFound(err) || IsForbidden(err) {
return nil
}
if err != nil {
return err
}
return errors.New("team reviews require an organization repository") return errors.New("team reviews require an organization repository")
} }
teams, err := (Teams{Client: s.Client}).ByName(ctx, owner) teams, err := (Teams{Client: s.Client}).ByName(ctx, owner)
if IsForbidden(err) { if IsForbidden(err) || IsNotFound(err) {
return nil return nil
} }
if err != nil { if err != nil {
+22
View File
@@ -95,3 +95,25 @@ func TestUnknownReviewerTeamRejectsBeforeUserRemoval(t *testing.T) {
_, err := (Reviews{Client: transport.New(server.URL, "", false), Repository: "owner/repo"}).ChangeRequests(context.Background(), 1, changes) _, err := (Reviews{Client: transport.New(server.URL, "", false), Repository: "owner/repo"}).ChangeRequests(context.Background(), 1, changes)
require.ErrorContains(t, err, `team "missing" is unavailable`) require.ErrorContains(t, err, `team "missing" is unavailable`)
} }
func TestReviewerTeamCheckSkippedForAHiddenPrivateOrganization(t *testing.T) {
var posted bool
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch {
case r.URL.Path == "/api/v1/orgs/owner" || r.URL.Path == "/api/v1/users/owner":
w.WriteHeader(http.StatusNotFound)
_, _ = w.Write([]byte(`{"message":"GetUserByName"}`))
case r.Method == http.MethodPost && r.URL.Path == "/api/v1/repos/owner/repo/pulls/1/requested_reviewers":
posted = true
w.WriteHeader(http.StatusCreated)
_, _ = w.Write([]byte(`[]`))
default:
_, _ = w.Write([]byte(`{"number":1}`))
}
}))
defer server.Close()
changes := ReviewRequestChanges{Add: ReviewRequestInput{Teams: []string{"devs"}}}
_, err := (Reviews{Client: transport.New(server.URL, "", false), Repository: "owner/repo"}).ChangeRequests(context.Background(), 1, changes)
require.NoError(t, err)
require.True(t, posted, "a non-member cannot see the organization, so the server decides")
}
+17
View File
@@ -250,6 +250,7 @@ func (s Teams) Create(ctx context.Context, org string, opts TeamOptions) (Team,
// permissions when changing access to all repositories. Gitea replaces the whole policy on edit. // permissions when changing access to all repositories. Gitea replaces the whole policy on edit.
func (s Teams) Edit(ctx context.Context, id int64, opts TeamOptions) (Team, error) { func (s Teams) Edit(ctx context.Context, id int64, opts TeamOptions) (Team, error) {
repositoryPolicy := opts.AllRepositories != nil && opts.Permission == nil repositoryPolicy := opts.AllRepositories != nil && opts.Permission == nil
changesPermission := opts.Permission != nil
if repositoryPolicy || (opts.Units != nil && !opts.ReplaceUnits) { if repositoryPolicy || (opts.Units != nil && !opts.ReplaceUnits) {
current, err := s.View(ctx, id) current, err := s.View(ctx, id)
if err != nil { if err != nil {
@@ -274,6 +275,22 @@ func (s Teams) Edit(ctx context.Context, id int64, opts TeamOptions) (Team, erro
opts.Units = units opts.Units = units
} }
} }
// Gitea 28 keeps an admin team admin when one request changes its permission and sets units_map,
// so change the permission alone first and then apply the units.
if changesPermission && opts.Units != nil {
first := TeamOptions{Name: opts.Name, Permission: opts.Permission}
if first.Name == nil {
current, err := s.View(ctx, id)
if err != nil {
return Team{}, err
}
first.Name = &current.Name
}
// TODO(sdk): EditTeamOption requires a name of at most 30 characters and a permission, and EditTeam should return the team.
if err := sendJSON(ctx, s.Client, http.MethodPatch, fmt.Sprintf("teams/%d", id), first, nil); err != nil {
return Team{}, err
}
}
var team gitea.Team var team gitea.Team
// TODO(sdk): EditTeamOption requires a name of at most 30 characters and a permission, and EditTeam should return the team. // TODO(sdk): EditTeamOption requires a name of at most 30 characters and a permission, and EditTeam should return the team.
if err := sendJSON(ctx, s.Client, http.MethodPatch, fmt.Sprintf("teams/%d", id), opts, &team); err != nil { if err := sendJSON(ctx, s.Client, http.MethodPatch, fmt.Sprintf("teams/%d", id), opts, &team); err != nil {
+19
View File
@@ -179,3 +179,22 @@ func TestTeamFindSearchesTheOrganization(t *testing.T) {
require.EqualError(t, err, `organization "nope" not found`) require.EqualError(t, err, `organization "nope" not found`)
require.Equal(t, []string{"devops", "devops", "", "", "DEV", "DEV"}, queries) require.Equal(t, []string{"devops", "devops", "", "", "DEV", "DEV"}, queries)
} }
func TestTeamEditSendsThePermissionBeforeTheUnits(t *testing.T) {
var bodies []map[string]any
client := newTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
require.Equal(t, "/api/v1/teams/5", r.URL.Path)
if r.Method == http.MethodGet {
_, _ = w.Write([]byte(`{"id":5,"name":"devs","permission":"admin"}`))
return
}
bodies = append(bodies, requestJSON(t, r))
_, _ = w.Write([]byte(`{"id":5,"name":"devs","permission":"write"}`))
}))
permission := "write"
_, err := (Teams{Client: client}).Edit(context.Background(), 5, TeamOptions{Permission: &permission, Units: map[string]string{"repo.wiki": "read"}, ReplaceUnits: true})
require.NoError(t, err)
require.Len(t, bodies, 2)
require.Equal(t, map[string]any{"name": "devs", "permission": "write"}, bodies[0], "Gitea 28 keeps an admin team admin when one request sets both")
require.Equal(t, map[string]any{"repo.wiki": "read"}, bodies[1]["units_map"])
}