novox/mesh-catalog
18
0
Fork 0
Code Issues Pull Requests 3 Actions Packages Projects Releases Wiki Activity

gitea: require no status on main, judge mesh/check in the merge tool instead (ADR 0534)

Browse Source 
The forge's required status was a second copy of the gate mesh-delivery and
the merge tool already keep; a stuck status writer held it too (issues 1886,
1891) and only a hand change at the forge let a fix merge. Main keeps no push,
the tools' merge whitelist and no administrator override. The merge tool
judges mesh/check whether or not the branch requires it, and on a branch that
requires none merges only a head the operator's Approve names. The protection
tool loses its status argument.
This commit is contained in:
jschoubben
2026-10-23 12:01:57 +02:00
parent 6c0024e43b
commit d2036f2a63
15 changed files with 307 additions and 300 deletions
Download Patch File Download Diff File Expand all files Collapse all files
modules/gitea/cmd/repositories/check_test.go
+76 -77
View File
@@ -5,130 +5,129 @@ import (
"testing"
)
// The tests of issue 1301 (ADR 0447 §1, §5; ADR 0503 §3, §5; lead decisions L4 and L11): main requires `mesh/check`
// alone in place of `mesh/merge-gate` and `mesh/repo-check`; a main that requires `mesh/repo-check` without
// `mesh/merge-gate` (hq) keeps it; every repository of the organisation whose main requires `mesh/merge-gate` is
// switched, listed in the value or not; no other status is dropped.
// The tests of ADR 0534 §1, §2 (the operator, 2026-10-23): the forge requires no status to merge. Main's rule keeps no
// push, no push whitelist, the tools' account alone in its merge whitelist and no administrator merging past it, and
// requires no status: the program writes it so on every repository it lists, and takes every required status off a
// rule covering main in every repository of the organisation it does not list. Nothing else of an unlisted
// repository's rule is sent.
func narrowedRule(contexts ...any) map[string]any {
return map[string]any{
"enable_push": false, "enable_push_whitelist": false, "push_whitelist_usernames": []any{},
"enable_merge_whitelist": true, "merge_whitelist_usernames": []any{tools},
"enable_status_check": true, "status_check_contexts": contexts,
"enable_status_check": len(contexts) > 0, "status_check_contexts": contexts,
"block_admin_merge_override": true,
}
}
// 1301-S1: a listed repository whose main requires the two old statuses is put right to `mesh/check` in their place,
// every other status kept; a second pass changes nothing.
func TestListedRepositorySwapsTheTwoStatusesForCheck(t *testing.T) {
// requiresNone is whether a rule as the fake forge keeps it requires no status.
func requiresNone(rule map[string]any) bool {
return rule["enable_status_check"] != true && len(strs(rule["status_check_contexts"])) == 0
}
// 0534-1: a listed repository whose main requires statuses has them all taken off, the rest of the rule kept; a
// second pass changes nothing.
func TestListedRepositoryRequiresNoStatus(t *testing.T) {
f := newFake()
repo := f.add("novox/mesh-delivery", map[string]map[string]any{mainRule: narrowedRule("mesh/merge-gate", "mesh/repo-check", "other/x")}, "main")
checked := f.add("novox/checked", map[string]map[string]any{mainRule: narrowedRule("mesh/check")}, "main")
s, _ := syncer(t, f)
o := run(t, s, Value{"mesh-delivery": {}})
got := strs(repo.Rules[mainRule]["status_check_contexts"])
if strings.Join(got, ",") != "other/x,mesh/check" {
t.Errorf("main requires %v, want other/x and mesh/check", got)
}
if r := result(o, "mesh-delivery"); r.Did != DidPutRight {
t.Errorf("outcome %+v, want put right", r)
o := run(t, s, Value{"mesh-delivery": {}, "checked": {}})
for name, r := range map[string]*fRepo{"mesh-delivery": repo, "checked": checked} {
rule := r.Rules[mainRule]
if !requiresNone(rule) {
t.Errorf("%s's main still requires %v", name, rule["status_check_contexts"])
}
if rule["enable_push"] != false || rule["enable_merge_whitelist"] != true ||
strings.Join(strs(rule["merge_whitelist_usernames"]), ",") != tools || rule["block_admin_merge_override"] != true {
t.Errorf("%s's main lost the rest of its rule: %v", name, rule)
}
if r := result(o, name); r.Did != DidPutRight {
t.Errorf("%s outcome %+v, want put right", name, r)
}
}
before := len(f.writes())
run(t, s, Value{"mesh-delivery": {}})
o = run(t, s, Value{"mesh-delivery": {}, "checked": {}})
if w := f.writes(); len(w) != before {
t.Errorf("a second pass wrote %v", w[before:])
}
if result(o, "checked").Did != DidUnchanged {
t.Errorf("second outcome %+v, want unchanged", result(o, "checked"))
}
}
// 1301-S1: a repository the program makes requires `mesh/check` alone from its first rule in the organisation on.
func TestNewRepositoryRequiresCheckAlone(t *testing.T) {
// 0534-1: a repository the program makes requires no status from its first rule in the organisation on.
func TestNewRepositoryRequiresNoStatus(t *testing.T) {
f := newFake()
s, _ := syncer(t, f)
run(t, s, Value{"fresh": {Description: "d"}})
rule := f.repos["novox/fresh"].Rules[mainRule]
if got := strs(rule["status_check_contexts"]); rule["enable_status_check"] != true || strings.Join(got, ",") != CheckStatus {
t.Errorf("a new repository's main requires %v, want %s alone", got, CheckStatus)
if !requiresNone(rule) {
t.Errorf("a new repository's main requires %v, want none", rule["status_check_contexts"])
}
if readme := f.repos["novox/fresh"].Files["README.md"]; !strings.Contains(readme, CheckStatus) ||
strings.Contains(readme, "mesh/merge-gate") || strings.Contains(readme, "mesh/repo-check") {
if readme := f.repos["novox/fresh"].Files["README.md"]; strings.Contains(readme, "mesh/merge-gate") || strings.Contains(readme, "mesh/repo-check") {
t.Errorf("the README names the statuses of before: %q", readme)
}
}
// 1301-S1: mesh/check is never dropped, and a rule that requires it beside a retired status loses the retired one.
func TestCheckIsKeptAndARetiredStatusBesideItGoes(t *testing.T) {
// 0534-1: a listed repository whose main requires nothing stays so, and is not written when the rest is right.
func TestListedRuleRequiringNothingIsRight(t *testing.T) {
f := newFake()
both := f.add("novox/both", map[string]map[string]any{mainRule: narrowedRule("mesh/check", "mesh/repo-check")}, "main")
right := f.add("novox/right", map[string]map[string]any{mainRule: narrowedRule("mesh/check")}, "main")
none := f.add("novox/none", map[string]map[string]any{mainRule: narrowedRule()}, "main")
s, _ := syncer(t, f)
o := run(t, s, Value{"both": {}, "right": {}})
if got := strs(both.Rules[mainRule]["status_check_contexts"]); strings.Join(got, ",") != CheckStatus {
t.Errorf("both requires %v, want %s alone", got, CheckStatus)
o := run(t, s, Value{"none": {}})
if !requiresNone(none.Rules[mainRule]) {
t.Errorf("none requires %v", none.Rules[mainRule]["status_check_contexts"])
}
if got := strs(right.Rules[mainRule]["status_check_contexts"]); strings.Join(got, ",") != CheckStatus {
t.Errorf("right requires %v", got)
if result(o, "none").Did != DidUnchanged {
t.Errorf("outcome %+v, want unchanged", result(o, "none"))
}
if result(o, "right").Did != DidUnchanged || result(o, "both").Did != DidPutRight {
t.Errorf("outcome %+v", o)
for _, w := range f.writes() {
t.Errorf("a right rule was written: %s", w)
}
}
// 1301-S1: a listed repository whose main requires nothing is given mesh/check, not the old two.
func TestListedRuleRequiringNothingGetsCheck(t *testing.T) {
f := newFake()
none := f.add("novox/none", map[string]map[string]any{mainRule: {"enable_push": false}}, "main")
s, _ := syncer(t, f)
run(t, s, Value{"none": {}})
if got := strs(none.Rules[mainRule]["status_check_contexts"]); strings.Join(got, ",") != CheckStatus {
t.Errorf("none requires %v, want %s", got, CheckStatus)
}
}
// 1301-S2 (ADR 0503 §5): a main that requires mesh/repo-check without mesh/merge-gate is judged by its own script
// alone (hq): it keeps mesh/repo-check and is not given mesh/check, listed or not.
func TestOwnCheckAloneIsKept(t *testing.T) {
// 0534-1: hq, whose main required mesh/repo-check alone, requires no status either, listed or not.
func TestOwnCheckAloneGoesToo(t *testing.T) {
f := newFake()
hq := f.add("novox/hq", map[string]map[string]any{mainRule: narrowedRule("mesh/repo-check")}, "main")
lab := f.add("novox/mesh-lab", map[string]map[string]any{mainRule: narrowedRule("mesh/repo-check")}, "main")
s, _ := syncer(t, f)
o := run(t, s, Value{"hq": {}})
for name, repo := range map[string]*fRepo{"hq": hq, "mesh-lab": lab} {
if got := strs(repo.Rules[mainRule]["status_check_contexts"]); strings.Join(got, ",") != "mesh/repo-check" {
t.Errorf("%s requires %v, want mesh/repo-check alone", name, got)
if !requiresNone(repo.Rules[mainRule]) {
t.Errorf("%s requires %v, want none", name, repo.Rules[mainRule]["status_check_contexts"])
}
if r := result(o, name); r.Did != DidPutRight {
t.Errorf("%s outcome %+v, want put right", name, r)
}
}
if result(o, "hq").Did != DidUnchanged {
t.Errorf("hq outcome %+v, want unchanged", result(o, "hq"))
}
for _, w := range f.writes() {
t.Errorf("a repository judged by its own check alone was written: %s", w)
}
}
// 1301-S3 (L4, L11): a repository the value does not list whose main requires mesh/merge-gate has the two old
// statuses swapped for mesh/check, and only its statuses: nothing else of its rule is sent. One that requires
// neither, or mesh/repo-check alone, is not written to.
func TestUnlistedRepositoryRequiringTheGateIsSwitched(t *testing.T) {
// 0534-1: a repository the value does not list whose rule covering main requires any status has them taken off, and
// only its statuses: nothing else of its rule is sent. One that requires none is not written to.
func TestUnlistedRepositoryRequiringAStatusHasItTakenOff(t *testing.T) {
f := newFake()
catalog := f.add("novox/mesh-catalog", map[string]map[string]any{mainRule: {
"enable_push": true, "enable_status_check": true, "status_check_contexts": []any{"mesh/merge-gate", "mesh/repo-check"},
"enable_push": true, "enable_status_check": true, "status_check_contexts": []any{"mesh/check"},
}}, "main")
toolsGo := f.add("novox/mesh-tools-go", map[string]map[string]any{mainRule: narrowedRule("mesh/merge-gate")}, "main")
f.add("novox/plain", map[string]map[string]any{mainRule: {"enable_push": false}}, "main")
f.add("novox/hq", map[string]map[string]any{mainRule: narrowedRule("mesh/repo-check")}, "main")
f.add("novox/unticked", map[string]map[string]any{mainRule: {"enable_push": false, "enable_status_check": false,
"status_check_contexts": []any{"mesh/check"}}}, "main")
f.add("novox/norule", nil, "main")
f.add("mesh-admin/elsewhere", map[string]map[string]any{mainRule: narrowedRule("mesh/merge-gate")}, "main")
s, _ := syncer(t, f)
o := run(t, s, Value{})
if got := strs(catalog.Rules[mainRule]["status_check_contexts"]); strings.Join(got, ",") != CheckStatus {
t.Errorf("mesh-catalog requires %v, want %s", got, CheckStatus)
if !requiresNone(catalog.Rules[mainRule]) {
t.Errorf("mesh-catalog requires %v, want none", catalog.Rules[mainRule]["status_check_contexts"])
}
if catalog.Rules[mainRule]["enable_push"] != true {
t.Errorf("an unlisted repository's rule was narrowed beyond its statuses: %v", catalog.Rules[mainRule])
t.Errorf("an unlisted repository's rule was changed beyond its statuses: %v", catalog.Rules[mainRule])
}
if got := strs(toolsGo.Rules[mainRule]["status_check_contexts"]); strings.Join(got, ",") != CheckStatus {
t.Errorf("mesh-tools-go requires %v, want %s", got, CheckStatus)
if !requiresNone(toolsGo.Rules[mainRule]) {
t.Errorf("mesh-tools-go requires %v, want none", toolsGo.Rules[mainRule]["status_check_contexts"])
}
want := []string{
"PATCH /api/v1/repos/novox/mesh-catalog/branch_protections/main",
@@ -142,7 +141,7 @@ func TestUnlistedRepositoryRequiringTheGateIsSwitched(t *testing.T) {
t.Errorf("%s outcome %+v, want put right", n, r)
}
}
for _, n := range []string{"plain", "hq", "norule", "elsewhere"} {
for _, n := range []string{"plain", "unticked", "norule", "elsewhere"} {
if r := result(o, n); r.Name != "" {
t.Errorf("%s is in the outcome though nothing was done: %+v", n, r)
}
@@ -154,27 +153,27 @@ func TestUnlistedRepositoryRequiringTheGateIsSwitched(t *testing.T) {
}
}
// 1301-S3: the organisation's repositories are read page by page, so one past the first page is switched too.
// 0534-1: the organisation's repositories are read page by page, so one past the first page is put right too.
func TestUnlistedRepositoriesAreReadPastTheFirstPage(t *testing.T) {
f := newFake()
f.pageSize = 2
for _, n := range []string{"a", "b", "c", "d", "e"} {
f.add("novox/"+n, map[string]map[string]any{mainRule: narrowedRule("mesh/merge-gate", "mesh/repo-check")}, "main")
f.add("novox/"+n, map[string]map[string]any{mainRule: narrowedRule("mesh/check")}, "main")
}
s, _ := syncer(t, f)
run(t, s, Value{})
for _, n := range []string{"a", "b", "c", "d", "e"} {
if got := strs(f.repos["novox/"+n].Rules[mainRule]["status_check_contexts"]); strings.Join(got, ",") != CheckStatus {
t.Errorf("%s requires %v", n, got)
if rule := f.repos["novox/"+n].Rules[mainRule]; !requiresNone(rule) {
t.Errorf("%s requires %v", n, rule["status_check_contexts"])
}
}
}
// 1301-S3: a refused swap is said with the repository and its step, and the pass goes on to the next.
func TestRefusedSwapIsSaid(t *testing.T) {
// 0534-1: a refused removal is said with the repository and its step, and the pass goes on to the next.
func TestRefusedRemovalIsSaid(t *testing.T) {
f := newFake()
f.add("novox/a", map[string]map[string]any{mainRule: narrowedRule("mesh/merge-gate")}, "main")
f.add("novox/b", map[string]map[string]any{mainRule: narrowedRule("mesh/merge-gate")}, "main")
f.add("novox/a", map[string]map[string]any{mainRule: narrowedRule("mesh/check")}, "main")
f.add("novox/b", map[string]map[string]any{mainRule: narrowedRule("mesh/check")}, "main")
f.refuse = func(method, path string) (int, string) {
if method == "PATCH" && strings.Contains(path, "/novox/a/") {
return 403, "no"
@@ -183,8 +182,8 @@ func TestRefusedSwapIsSaid(t *testing.T) {
}
s, _ := syncer(t, f)
o := run(t, s, Value{})
if r := result(o, "a"); r.Did != DidRefused || r.Step != StepSwap {
t.Errorf("a: %+v, want refused at %s", r, StepSwap)
if r := result(o, "a"); r.Did != DidRefused || r.Step != StepNoStatus {
t.Errorf("a: %+v, want refused at %s", r, StepNoStatus)
}
if r := result(o, "b"); r.Did != DidPutRight {
t.Errorf("b: %+v, want put right", r)
modules/gitea/cmd/repositories/main.go
+2 -2
View File
@@ -3,8 +3,8 @@
// again whenever the value changes: it reads the file the mesh renders from that value and, with the setup login,
// makes every listed repository that does not exist under that login's own account, protects its main, writes
// main's first commit, narrows main's rule and only then moves it into the organisation; one that exists only has
// its main rule put right, requiring mesh/check in place of the two statuses it replaces (issue 1301). A repository
// the value does not list is touched only to swap those two for mesh/check when its main requires mesh/merge-gate. It keeps what it did, per
// its main rule put right, requiring no status (novox/hq ADR 0534 §1). A repository the value does not list is
// touched only to take the required statuses off a rule covering its main. It keeps what it did, per
// repository, in the outcome file, says each change on stdout in one line and never a secret, and exits 1 when a
// step was refused, its last stderr lines naming each (outcome.go Report): the node-engine's gate on a run-once
// step (ADR 0136) reports it and runs it again on every apply until it passes.
modules/gitea/cmd/repositories/manifest_test.go
+9
View File
@@ -105,3 +105,12 @@ func TestManifestDeclaresTheValueAndItsTrigger(t *testing.T) {
t.Errorf("repositories is declared unlike agent-read-teams: %v vs %v", v, teams)
}
}
func has(list []string, want string) bool {
for _, s := range list {
if s == want {
return true
}
}
return false
}
modules/gitea/cmd/repositories/override_test.go
+4 -4
View File
@@ -5,7 +5,7 @@ import (
"testing"
)
// 1141/AC1: a repository the program makes gets main's rule with an administrator's merge past a status refused,
// 1141/AC1: a repository the program makes gets main's rule with an administrator's merge past it refused,
// as every repository's main has had since 2026-10-07.
func TestNewRepositoryRefusesTheAdministratorsOverride(t *testing.T) {
f := newFake()
@@ -18,7 +18,7 @@ func TestNewRepositoryRefusesTheAdministratorsOverride(t *testing.T) {
}
// 1141/AC2: a narrowed main rule that still lets an administrator override is not right: it is put right, its
// statuses kept (the two old ones as mesh/check, issue 1301), and a second pass changes nothing.
// statuses taken off (ADR 0534 §1), and a second pass changes nothing.
func TestNarrowedRuleLettingTheAdministratorOverrideIsPutRight(t *testing.T) {
f := newFake()
repo := f.add("novox/mesh-delivery", map[string]map[string]any{mainRule: {
@@ -33,8 +33,8 @@ func TestNarrowedRuleLettingTheAdministratorOverrideIsPutRight(t *testing.T) {
if rule["block_admin_merge_override"] != true {
t.Errorf("the administrator's override is still allowed: %v", rule)
}
if c := strs(rule["status_check_contexts"]); strings.Join(c, ",") != "other/x,mesh/check" {
t.Errorf("statuses %v, want other/x kept and mesh/check for the two it replaces (issue 1301)", c)
if !requiresNone(rule) {
t.Errorf("statuses %v, want none (ADR 0534 §1)", rule["status_check_contexts"])
}
if r := result(o, "mesh-delivery"); r.Did != DidPutRight {
t.Errorf("outcome %+v, want put right", r)
modules/gitea/cmd/repositories/repositories.go
+32 -87
View File
@@ -26,46 +26,11 @@ const (
MainBranch = "main"
)
// CheckStatus is the one status main requires (ADR 0447 §1, issue 1301): the checker's verdict on a pull request's
// head merged onto its trunk's head. It replaces GateStatus and OwnCheckStatus, and is never dropped.
const CheckStatus = "mesh/check"
// GateStatus and OwnCheckStatus are the two statuses CheckStatus replaces: the controller's merge gate and the
// repository's own merge-check.sh (ADR 0238 §4, ADR 0428 §5 before ADR 0447 §1).
const (
GateStatus = "mesh/merge-gate"
OwnCheckStatus = "mesh/repo-check"
)
// wanted is what a main rule requiring have should require: have with GateStatus and OwnCheckStatus replaced by
// CheckStatus, every other status kept in its order (ADR 0447 §1). A rule requiring OwnCheckStatus with neither
// GateStatus nor CheckStatus is judged by its own script alone, as hq is until issue 1296 settles how hq is
// checked (ADR 0503 §5): the checker writes no status there (ADR 0484 §8), so it is left as it is. This reads the
// rule, not a list of names (lead decision L11 of R4e).
func wanted(have []string) []string {
if !has(have, GateStatus) && !has(have, CheckStatus) && has(have, OwnCheckStatus) {
return have
}
out := []string{}
for _, s := range have {
if s != GateStatus && s != OwnCheckStatus && s != CheckStatus {
out = append(out, s)
}
}
return append(out, CheckStatus)
}
func has(list []string, want string) bool {
for _, s := range list {
if s == want {
return true
}
}
return false
}
func sameList(a, b []string) bool {
return strings.Join(a, "\x00") == strings.Join(b, "\x00")
// noStatus is what main's rule says of statuses: none is required to merge (novox/hq ADR 0534 §1, the operator,
// 2026-10-23). The checks still run and report; mesh-delivery asks the operator's Approve on a passing verdict and the
// merge tool judges it at the merge (ADR 0534 §3, §4). The forge enforces none of them.
func noStatus() map[string]any {
return map[string]any{"enable_status_check": false, "status_check_contexts": []string{}}
}
// The steps of one repository, as an outcome names a refused one (ADR 0428 §6).
@@ -82,8 +47,8 @@ const (
StepRuleAfterMove = "rule-after-move"
// StepList reads the organisation's repositories, for those the value does not list (issue 1301).
StepList = "list"
// StepSwap swaps the two old statuses for CheckStatus on a repository the value does not list (issue 1301).
StepSwap = "swap"
// StepNoStatus takes the required statuses off main's rule of a repository the value does not list (ADR 0534 §1).
StepNoStatus = "no-status"
)
// What a pass did with one repository.
@@ -203,7 +168,7 @@ func (s *Syncer) Sync(ctx context.Context, v Value) (Outcome, error) {
if r.Did == DidRefused {
s.say("refused: %s/%s at the step %s: %s", s.Org, r.Name, r.Step, r.Said)
} else {
s.say("%s: %s/%s (%s in place of %s and %s)", r.Did, s.Org, r.Name, CheckStatus, GateStatus, OwnCheckStatus)
s.say("%s: %s/%s (main requires no status)", r.Did, s.Org, r.Name)
}
out.Repositories = append(out.Repositories, r)
}
@@ -213,12 +178,10 @@ func (s *Syncer) Sync(ctx context.Context, v Value) (Outcome, error) {
// orgPage is how many repositories one read of the organisation's list asks for.
const orgPage = 50
// switchUnlisted swaps the two old statuses for CheckStatus on main's rule of every repository of the organisation
// the value does not list whose main requires GateStatus (issue 1301, lead decisions L4 and L11 of R4e): the
// repositories the merge gate judges are switched in the delivery that has the checker write mesh/check (ADR 0447
// §5), with no list kept by hand. Only the statuses of a rule covering main are sent; nothing else of a rule the
// value does not hold is changed (ADR 0428 §7 for all of it but the statuses the mesh itself requires). A main that
// does not require GateStatus gets no call. It answers only what it did: a repository switched, or refused.
// switchUnlisted takes every required status off a rule covering main in every repository of the organisation the
// value does not list (ADR 0534 §1): the forge requires no status to merge anywhere, with no list kept by hand. Only
// the statuses are sent; nothing else of a rule the value does not hold is changed (ADR 0428 §7). A main whose rules
// require no status gets no call. It answers only what it did: a repository put right, or refused.
func (s *Syncer) switchUnlisted(ctx context.Context, v Value) []Result {
var names []string
for page := 1; ; page++ {
@@ -248,13 +211,12 @@ func (s *Syncer) switchUnlisted(ctx context.Context, v Value) []Result {
}
var did *Result
for _, r := range rules {
if ok, _ := path.Match(ruleName(r), MainBranch); !ok || !has(required(r), GateStatus) {
if ok, _ := path.Match(ruleName(r), MainBranch); !ok || len(required(r)) == 0 {
continue
}
body := map[string]any{"enable_status_check": true, "status_check_contexts": wanted(required(r))}
if err := s.Forge.SendJSON(ctx, http.MethodPatch,
repoAPI(s.Org, n)+"/branch_protections/"+url.PathEscape(ruleName(r)), body, nil); err != nil {
did = &Result{Name: n, Did: DidRefused, Step: StepSwap, Said: err.Error()}
repoAPI(s.Org, n)+"/branch_protections/"+url.PathEscape(ruleName(r)), noStatus(), nil); err != nil {
did = &Result{Name: n, Did: DidRefused, Step: StepNoStatus, Said: err.Error()}
break
}
did = &Result{Name: n, Did: DidPutRight}
@@ -351,7 +313,7 @@ func (s *Syncer) one(ctx context.Context, name string, r Repo) Result {
}
if rule == nil || !isRight(rule, s.Tools) {
if err := s.Forge.SendJSON(ctx, http.MethodPatch, repoAPI(s.Admin, name)+"/branch_protections/"+MainBranch,
narrowed(rule, s.Tools), nil); err != nil {
narrowed(s.Tools), nil); err != nil {
return refused(name, StepNarrow, err)
}
}
@@ -415,24 +377,6 @@ func strList(v any) []string {
return out
}
// statuses is what the rules require together, as wanted makes it.
func statuses(rules ...rule) []string {
seen := map[string]bool{}
var out []string
add := func(s string) {
if !seen[s] {
seen[s] = true
out = append(out, s)
}
}
for _, r := range rules {
for _, s := range required(r) {
add(s)
}
}
return wanted(out)
}
// required is what a rule requires: its statuses when it checks them, else none.
func required(r rule) []string {
if r == nil || r["enable_status_check"] != true {
@@ -441,19 +385,22 @@ func required(r rule) []string {
return strList(r["status_check_contexts"])
}
// narrowed is main's rule of ADR 0398 §2 (protection.ts mainRuleBody): no push, only the tools' account merges,
// requiring the statuses (ADR 0428 §5), and no administrator merging past them, as every repository's main has been
// since 2026-10-07 (issue 1141). A field the forge keeps beside these is left as it is.
func narrowed(r rule, toolsAccount string, also ...rule) map[string]any {
return map[string]any{
// narrowed is main's rule of ADR 0398 §2 (protection.ts mainRuleBody): no push, only the tools' account merges, no
// administrator merging past the rule, as every repository's main has been since 2026-10-07 (issue 1141), and no
// status required (ADR 0534 §1). A field the forge keeps beside these is left as it is.
func narrowed(toolsAccount string) map[string]any {
body := map[string]any{
"enable_push": false, "enable_push_whitelist": false, "push_whitelist_usernames": []string{},
"enable_merge_whitelist": true, "merge_whitelist_usernames": []string{toolsAccount},
"enable_status_check": true, "status_check_contexts": statuses(append([]rule{r}, also...)...),
"block_admin_merge_override": true,
}
for k, v := range noStatus() {
body[k] = v
}
return body
}
// isRight says whether a rule already is narrowed and requires what wanted makes of it.
// isRight says whether a rule already is narrowed and requires no status.
func isRight(r rule, toolsAccount string) bool {
if r["enable_push"] != false || r["enable_push_whitelist"] == true || r["enable_merge_whitelist"] != true {
return false
@@ -461,15 +408,13 @@ func isRight(r rule, toolsAccount string) bool {
if m := strList(r["merge_whitelist_usernames"]); len(m) != 1 || m[0] != toolsAccount {
return false
}
have := strList(r["status_check_contexts"])
return r["enable_status_check"] == true && len(have) > 0 && sameList(have, wanted(have)) &&
return r["enable_status_check"] != true && len(strList(r["status_check_contexts"])) == 0 &&
r["block_admin_merge_override"] == true
}
// putRight makes main's rule in an organisation's repository the narrowed one, keeping every status it requires but
// the two CheckStatus replaces (ADR 0447 §1).
// A repository with no rule named main gets one, carrying what a glob rule covering main asks (client.ts
// ensureMainMerges, issue 948). It takes away no other status and never unprotects (ADR 0428 §5, §7).
// putRight makes main's rule in an organisation's repository the narrowed one, requiring no status (ADR 0534 §1).
// A repository with no rule named main gets one, carrying the approvals a glob rule covering main asks (client.ts
// ensureMainMerges, issue 948). It never unprotects (ADR 0428 §7).
func (s *Syncer) putRight(ctx context.Context, owner, name string) (bool, error) {
rules, err := s.rules(ctx, owner, name)
if err != nil {
@@ -480,7 +425,7 @@ func (s *Syncer) putRight(ctx context.Context, owner, name string) (bool, error)
return false, nil
}
return true, s.Forge.SendJSON(ctx, http.MethodPatch, repoAPI(owner, name)+"/branch_protections/"+MainBranch,
narrowed(own, s.Tools), nil)
narrowed(s.Tools), nil)
}
var covering []rule
for _, r := range rules {
@@ -488,7 +433,7 @@ func (s *Syncer) putRight(ctx context.Context, owner, name string) (bool, error)
covering = append(covering, r)
}
}
body := narrowed(nil, s.Tools, covering...)
body := narrowed(s.Tools)
body["rule_name"] = MainBranch
approvals := 0.0
for _, r := range covering {
modules/gitea/cmd/repositories/repositories_test.go
+22 -26
View File
@@ -19,7 +19,7 @@ const mainRule = "main"
// 1124/AC2: a listed repository that does not exist is made under the setup account, given main's rule letting that
// login alone push, given its first commit, narrowed to no push and the tools' merge requiring mesh/check (1124/AC5
// as issue 1301 changes it), and only then moved into the organisation. Nothing reaches the organisation before the
// narrowed rule, and nothing else is pushed to main.
// narrowed rule, and nothing else is pushed to main. Main requires no status (ADR 0534 §1).
func TestNewRepositoryIsMadeInOrder(t *testing.T) {
f := newFake()
s, _ := syncer(t, f)
@@ -66,9 +66,8 @@ func TestNewRepositoryIsMadeInOrder(t *testing.T) {
if rule["enable_merge_whitelist"] != true || strings.Join(strs(rule["merge_whitelist_usernames"]), ",") != tools {
t.Errorf("main's merge whitelist is not the tools' account alone: %v", rule)
}
checks := strs(rule["status_check_contexts"])
if rule["enable_status_check"] != true || strings.Join(checks, ",") != CheckStatus {
t.Errorf("main does not require mesh/check alone (1124/AC5, issue 1301): %v", rule)
if !requiresNone(rule) {
t.Errorf("main requires a status (ADR 0534 §1): %v", rule)
}
if r := result(o, "mesh-delivery"); r.Did != DidMade || r.Step != "" {
t.Errorf("outcome %+v, want made", r)
@@ -87,7 +86,7 @@ func TestNewRepositoryIsMadeInOrder(t *testing.T) {
// 1124/AC2: the forge drops from a merge whitelist a user without access to the repository, and under the setup
// account the tools' account has none; so once the repository is in the organisation its main rule is read back and
// put right: the tools' account the only one that merges, mesh/check required.
// put right: the tools' account the only one that merges, no status required (ADR 0534 §1).
func TestOrganisationRuleListsTheToolsAfterTheMove(t *testing.T) {
f := newFake()
s, _ := syncer(t, f)
@@ -103,9 +102,8 @@ func TestOrganisationRuleListsTheToolsAfterTheMove(t *testing.T) {
if rule["enable_push"] != false {
t.Errorf("after the move main takes a push: %v", rule)
}
checks := strs(rule["status_check_contexts"])
if rule["enable_status_check"] != true || strings.Join(checks, ",") != CheckStatus {
t.Errorf("after the move main does not require mesh/check alone: %v", rule)
if !requiresNone(rule) {
t.Errorf("after the move main requires a status (ADR 0534 §1): %v", rule)
}
if r := result(o, "mesh-delivery"); r.Did != DidMade {
t.Errorf("outcome %+v, want made", r)
@@ -136,8 +134,7 @@ func TestAdminRuleLetsTheSetupLoginAlonePush(t *testing.T) {
}
// 1124/AC3: a listed repository that exists in the organisation is not made again, only its main rule put right;
// a rule still letting the setup login push is narrowed, its statuses kept but the two mesh/check replaces (1124/AC5,
// issue 1301).
// a rule still letting the setup login push is narrowed, and requires no status (ADR 0534 §1).
func TestExistingOrganisationRepositoryIsOnlyPutRight(t *testing.T) {
f := newFake()
hq := f.add("novox/hq", map[string]map[string]any{mainRule: {
@@ -157,12 +154,11 @@ func TestExistingOrganisationRepositoryIsOnlyPutRight(t *testing.T) {
if rule["enable_push"] != false || strings.Join(strs(rule["merge_whitelist_usernames"]), ",") != tools {
t.Errorf("hq's rule not narrowed: %v", rule)
}
checks := strs(rule["status_check_contexts"])
if strings.Join(checks, ",") != "other/x,mesh/check" {
t.Errorf("hq's rule requires %v, want other/x and mesh/check", checks)
if !requiresNone(rule) {
t.Errorf("hq's rule requires %v, want none (ADR 0534 §1)", rule["status_check_contexts"])
}
made := bare.Rules[mainRule]
if made == nil || made["enable_push"] != false || strings.Join(strs(made["status_check_contexts"]), ",") != CheckStatus {
if made == nil || made["enable_push"] != false || !requiresNone(made) {
t.Errorf("bare's rule: %v", made)
}
if hq.Commits != 0 || bare.Commits != 0 {
@@ -212,33 +208,33 @@ func TestRepositoryUnderTheSetupAccountIsFinished(t *testing.T) {
}
}
// 1124/AC5 as issue 1301 changes it: putting a rule right adds mesh/check when the rule requires mesh/merge-gate,
// and keeps every other status there; a rule requiring mesh/repo-check alone keeps it (ADR 0503 §5).
func TestStatusesAreAddedAndNeverDropped(t *testing.T) {
// ADR 0534 §1: putting a rule right takes every required status off it, whichever they are, and a rule already right
// is not written.
func TestStatusesAreTakenOff(t *testing.T) {
f := newFake()
only := f.add("novox/only", map[string]map[string]any{mainRule: {
"enable_push": false, "enable_merge_whitelist": true, "merge_whitelist_usernames": []any{tools},
"enable_status_check": true, "status_check_contexts": []any{"mesh/repo-check"},
}}, "main")
more := f.add("novox/more", map[string]map[string]any{mainRule: {
right := f.add("novox/right", map[string]map[string]any{mainRule: {
"enable_push": false, "enable_merge_whitelist": true, "merge_whitelist_usernames": []any{tools},
"enable_status_check": true, "status_check_contexts": []any{"mesh/delivery", "mesh/check"},
"enable_status_check": false, "status_check_contexts": []any{},
"block_admin_merge_override": true, // right since issue 1141
}}, "main")
s, _ := syncer(t, f)
o := run(t, s, Value{"only": {}, "more": {}})
if c := strs(only.Rules[mainRule]["status_check_contexts"]); strings.Join(c, ",") != "mesh/repo-check" {
t.Errorf("only: %v", c)
o := run(t, s, Value{"only": {}, "right": {}})
if !requiresNone(only.Rules[mainRule]) {
t.Errorf("only: %v", only.Rules[mainRule]["status_check_contexts"])
}
if c := strs(more.Rules[mainRule]["status_check_contexts"]); len(c) != 2 || !has(c, "mesh/delivery") {
t.Errorf("more lost a status: %v", c)
if !requiresNone(right.Rules[mainRule]) {
t.Errorf("right: %v", right.Rules[mainRule]["status_check_contexts"])
}
for _, w := range f.writes() {
if strings.Contains(w, "/more/") {
if strings.Contains(w, "/right/") {
t.Errorf("a right rule was written: %s", w)
}
}
if result(o, "more").Did != DidUnchanged || result(o, "only").Did != DidPutRight {
if result(o, "right").Did != DidUnchanged || result(o, "only").Did != DidPutRight {
t.Errorf("outcome %+v", o)
}
}