Revert "Swap hq's repo-check-alone rule to mesh/check like every main: nothing writes repo-check once build-agent goes (issue 1296, E1a)"

This reverts commit e4ec060ac7.
This commit is contained in:
2026-10-23 16:10:09 +02:00
parent 612486be18
commit fa39738981
3 changed files with 28 additions and 25 deletions
+15 -16
View File
@@ -7,7 +7,7 @@ import (
// 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) is swapped too (ADR 0527 §2); every repository of the organisation whose main requires either is
// `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.
func narrowedRule(contexts ...any) map[string]any {
@@ -84,27 +84,30 @@ func TestListedRuleRequiringNothingGetsCheck(t *testing.T) {
}
}
// 1296/E1 (ADR 0527 §2): a main that requires mesh/repo-check alone (hq) is given mesh/check in its place, listed or
// not, since nothing writes mesh/repo-check once build-agent is gone.
func TestOwnCheckAloneIsSwapped(t *testing.T) {
// 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) {
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, ",") != CheckStatus {
t.Errorf("%s requires %v, want %s alone", name, got, CheckStatus)
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 result(o, "hq").Did == DidUnchanged {
t.Errorf("hq outcome %+v, want a change", result(o, "hq"))
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 is not written to.
// neither, or mesh/repo-check alone, is not written to.
func TestUnlistedRepositoryRequiringTheGateIsSwitched(t *testing.T) {
f := newFake()
catalog := f.add("novox/mesh-catalog", map[string]map[string]any{mainRule: {
@@ -112,7 +115,7 @@ func TestUnlistedRepositoryRequiringTheGateIsSwitched(t *testing.T) {
}}, "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")
hq := f.add("novox/hq", map[string]map[string]any{mainRule: narrowedRule("mesh/repo-check")}, "main")
f.add("novox/hq", map[string]map[string]any{mainRule: narrowedRule("mesh/repo-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)
@@ -127,23 +130,19 @@ func TestUnlistedRepositoryRequiringTheGateIsSwitched(t *testing.T) {
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 got := strs(hq.Rules[mainRule]["status_check_contexts"]); strings.Join(got, ",") != CheckStatus {
t.Errorf("hq requires %v, want %s", got, CheckStatus)
}
want := []string{
"PATCH /api/v1/repos/novox/hq/branch_protections/main",
"PATCH /api/v1/repos/novox/mesh-catalog/branch_protections/main",
"PATCH /api/v1/repos/novox/mesh-tools-go/branch_protections/main",
}
if got := f.writes(); strings.Join(got, "\n") != strings.Join(want, "\n") {
t.Errorf("writes:\n%s\nwant:\n%s", strings.Join(got, "\n"), strings.Join(want, "\n"))
}
for _, n := range []string{"hq", "mesh-catalog", "mesh-tools-go"} {
for _, n := range []string{"mesh-catalog", "mesh-tools-go"} {
if r := result(o, n); r.Did != DidPutRight {
t.Errorf("%s outcome %+v, want put right", n, r)
}
}
for _, n := range []string{"plain", "norule", "elsewhere"} {
for _, n := range []string{"plain", "hq", "norule", "elsewhere"} {
if r := result(o, n); r.Name != "" {
t.Errorf("%s is in the outcome though nothing was done: %+v", n, r)
}
+11 -7
View File
@@ -38,10 +38,14 @@ const (
)
// 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 alone, as hq's
// did, is swapped too: the checker judges hq like any repository and nothing writes OwnCheckStatus once the merge
// check's second layer is gone (ADR 0527 §2, issue 1296). This reads the rule, not a list of names (lead decision L11 of R4e).
// 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 {
@@ -210,11 +214,11 @@ func (s *Syncer) Sync(ctx context.Context, v Value) (Outcome, error) {
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 or OwnCheckStatus (issue 1301, lead decisions L4 and L11 of R4e): the
// 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; hq's, ADR 0527 §2), with no list kept by hand. Only the statuses of a rule covering main are sent; nothing else of a rule the
// §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
// requires neither gets no call. It answers only what it did: a repository switched, or refused.
// does not require GateStatus gets no call. It answers only what it did: a repository switched, or refused.
func (s *Syncer) switchUnlisted(ctx context.Context, v Value) []Result {
var names []string
for page := 1; ; page++ {
@@ -244,7 +248,7 @@ 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) || has(required(r), OwnCheckStatus)) {
if ok, _ := path.Match(ruleName(r), MainBranch); !ok || !has(required(r), GateStatus) {
continue
}
body := map[string]any{"enable_status_check": true, "status_check_contexts": wanted(required(r))}
@@ -212,7 +212,7 @@ 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 is given mesh/check (ADR 0527 §2).
// and keeps every other status there; a rule requiring mesh/repo-check alone keeps it (ADR 0503 §5).
func TestStatusesAreAddedAndNeverDropped(t *testing.T) {
f := newFake()
only := f.add("novox/only", map[string]map[string]any{mainRule: {
@@ -226,7 +226,7 @@ func TestStatusesAreAddedAndNeverDropped(t *testing.T) {
}}, "main")
s, _ := syncer(t, f)
o := run(t, s, Value{"only": {}, "more": {}})
if c := strs(only.Rules[mainRule]["status_check_contexts"]); strings.Join(c, ",") != CheckStatus {
if c := strs(only.Rules[mainRule]["status_check_contexts"]); strings.Join(c, ",") != "mesh/repo-check" {
t.Errorf("only: %v", c)
}
if c := strs(more.Rules[mainRule]["status_check_contexts"]); len(c) != 2 || !has(c, "mesh/delivery") {