Merge pull request 'Ask a refused merge again when its checks turn green on the forge (issue 1144)' (#368) from fix/1144-merge-asked-again-on-green-status into main

The tracker runs with the manifest's owners default; a test now holds the
default and the template's copy to one list.

modules/gitea/cmd/repositories/manifest_test.go

@@ -3,6 +3,7 @@ package main
 import (
 	"encoding/json"
 	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 )
@@ -35,14 +36,33 @@ func TestManifestDeclaresTheValueAndItsTrigger(t *testing.T) {
 	}
 	var file, process map[string]any
 	for _, r := range m.Resources {
-		for _, k := range strs(r["takes-config"]) {
-			if k == ConfigKey {
-				file = r
-			}
+		if r["id"] == "repositories-file" {
+			file = r
 		}
 	}
 	if file == nil || file["type"] != "file" {
-		t.Fatalf("no file takes the value %s", ConfigKey)
+		t.Fatalf("no file repositories-file")
+	}
+	// 1128/AC1: a plain template the provisioner client fills, so an approved value lands at once: a file the
+	// controller merges (merge, takes-config) takes a config change only on the node's next send (ADR 0348 §10).
+	if file["merge"] != nil || file["takes-config"] != nil {
+		t.Errorf("repositories-file is merged by the controller (merge %v, takes-config %v), want a plain template",
+			file["merge"], file["takes-config"])
+	}
+	text, err := os.ReadFile("../../" + file["template"].(string))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !strings.Contains(string(text), "${config:"+ConfigKey+"}") {
+		t.Fatalf("the template %s does not say ${config:%s}: %s", file["template"], ConfigKey, text)
+	}
+	filled := filepath.Join(t.TempDir(), "repositories.json")
+	value := `{"mesh-delivery":{"private":false,"description":"d"}}`
+	if err := os.WriteFile(filled, []byte(strings.ReplaceAll(string(text), "${config:"+ConfigKey+"}", value)), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	if v, err := ReadValue(filled); err != nil || len(v) != 1 || v["mesh-delivery"].Description != "d" {
+		t.Errorf("the filled template reads as %v, %v; want mesh-delivery", v, err)
 	}
 	for _, r := range m.Resources {
 		if r["type"] == "process" && has(strs(r["restart-on"]), file["id"].(string)) {

modules/gitea/cmd/repositories/override_test.go

@@ -0,0 +1,49 @@
+package main
+
+import (
+	"strings"
+	"testing"
+)
+
+// 1141/AC1: a repository the program makes gets main's rule with an administrator's merge past a status refused,
+// as every repository's main has had since 2026-10-07.
+func TestNewRepositoryRefusesTheAdministratorsOverride(t *testing.T) {
+	f := newFake()
+	s, _ := syncer(t, f)
+	run(t, s, Value{"mesh-delivery": {Description: "d"}})
+	rule := f.repos["novox/mesh-delivery"].Rules[mainRule]
+	if rule["block_admin_merge_override"] != true {
+		t.Errorf("main lets an administrator merge past its statuses: %v", rule)
+	}
+}
+
+// 1141/AC2: a narrowed main rule that still lets an administrator override is not right: it is put right, its
+// statuses kept, 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: {
+		"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": []any{"mesh/merge-gate", "mesh/repo-check", "other/x"},
+		"block_admin_merge_override": false,
+	}}, "main")
+	s, _ := syncer(t, f)
+	o := run(t, s, Value{"mesh-delivery": {Description: "d"}})
+	rule := repo.Rules[mainRule]
+	if rule["block_admin_merge_override"] != true {
+		t.Errorf("the administrator's override is still allowed: %v", rule)
+	}
+	for _, c := range []string{"mesh/merge-gate", "mesh/repo-check", "other/x"} {
+		if !has(strs(rule["status_check_contexts"]), c) {
+			t.Errorf("a status was dropped: %v", rule["status_check_contexts"])
+		}
+	}
+	if r := result(o, "mesh-delivery"); r.Did != DidPutRight {
+		t.Errorf("outcome %+v, want put right", r)
+	}
+	before := len(f.writes())
+	run(t, s, Value{"mesh-delivery": {Description: "d"}})
+	if got := f.writes(); len(got) != before {
+		t.Errorf("a second pass wrote %s", strings.Join(got[before:], ", "))
+	}
+}

modules/gitea/cmd/repositories/repositories.go

@@ -333,12 +333,14 @@ func statuses(rules ...rule) []string {
 }
 
 // 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). A field the forge keeps beside these is left as it is.
+// 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{
 		"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,
 	}
 }
 
@@ -350,7 +352,8 @@ func isRight(r rule, toolsAccount string) bool {
 	if m := strList(r["merge_whitelist_usernames"]); len(m) != 1 || m[0] != toolsAccount {
 		return false
 	}
-	return r["enable_status_check"] == true && hasAll(strList(r["status_check_contexts"]))
+	return r["enable_status_check"] == true && hasAll(strList(r["status_check_contexts"])) &&
+		r["block_admin_merge_override"] == true
 }
 
 func hasAll(have []string) bool {
@@ -394,9 +397,6 @@ func (s *Syncer) putRight(ctx context.Context, owner, name string) (bool, error)
 		if n, _ := r["required_approvals"].(float64); n > approvals {
 			approvals = n
 		}
-		if r["block_admin_merge_override"] == true {
-			body["block_admin_merge_override"] = true
-		}
 	}
 	if approvals > 0 {
 		body["required_approvals"] = approvals

modules/gitea/cmd/repositories/repositories_test.go

@@ -225,6 +225,7 @@ func TestStatusesAreAddedAndNeverDropped(t *testing.T) {
 	more := f.add("novox/more", 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/merge-gate", "mesh/repo-check", "mesh/delivery"},
+		"block_admin_merge_override": true, // right since issue 1141
 	}}, "main")
 	s, _ := syncer(t, f)
 	o := run(t, s, Value{"only": {}, "more": {}})

modules/gitea/files/repositories.json

@@ -1 +1 @@
-{}
+{"repositories": ${config:repositories}}

modules/gitea/module.json

@@ -322,11 +322,7 @@
       "type": "file",
       "path": "${dir:mesh-state}/repositories.json",
       "mode": "0600",
-      "template": "files/repositories.json",
-      "merge": "json",
-      "takes-config": [
-        "repositories"
-      ]
+      "template": "files/repositories.json"
     },
     {
       "id": "consumers",

modules/mesh-delivery/cmd/mesh-delivery/events.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"slices"
+	"strings"
 
 	"git.novox.be/novox/mesh-sdk/go/asks"
 )
@@ -51,8 +52,34 @@ func (h *Holder) AskDecided(id, state string, w *asks.Warrant) {
 // CommitStatus takes a commit status set on the forge: one of the merge checks (mesh/merge-gate, mesh/repo-check) says a
 // check started, answered, or was put back, so the forge's open pull requests are read to see which heads still wait on
 // one (SeeOpenPulls). A status of any other context shows nothing the merge needs.
+//
+// A merge check turning success for the head an approved, ready, not-yet-merged delivery's merge was refused at is
+// itself an event that decides whether it may merge (issue 1144): the merge tool may have refused it on a check still
+// pending, which this status answers. The merge is asked again at once (mergeAgainNow), under the lock, so the next
+// look asks it; no timer does this (issue 1115). A status of another context, another commit or a state other than
+// success changes nothing.
 func (h *Holder) CommitStatus(e ForgeStatusEvent) {
-	if slices.Contains(mergeChecks, e.Context) {
-		h.wantForge()
+	if !slices.Contains(mergeChecks, e.Context) {
+		return
+	}
+	h.wantForge()
+	if e.State != "success" {
+		return
+	}
+	h.mu.Lock()
+	defer h.mu.Unlock()
+	repository := strings.ToLower(e.Owner + "/" + e.Repo)
+	for _, d := range h.deliveries {
+		if !strings.EqualFold(d.Repository, repository) || d.Ready == nil || d.Ready.Merged != nil || d.MergedAs != "" ||
+			d.State.Final() {
+			continue
+		}
+		head := d.Commit
+		if sha := d.Ready.Heads[d.ID]; sha != "" {
+			head = sha
+		}
+		if sameCommit(head, e.SHA) {
+			h.mergeAgainNow(d) // its approved head's check turned success on the forge: the merge is asked again at once
+		}
 	}
 }

modules/mesh-delivery/cmd/mesh-delivery/ready.go

@@ -427,7 +427,8 @@ func (h *Holder) readyLines(now time.Time) []StalledLine {
 		case r.RefusedSince != nil && now.Sub(*r.RefusedSince) >= readyRefusedFor:
 			out = append(out, StalledLine{ID: d.ID, Number: d.Number, State: d.State,
 				For: now.Sub(*r.RefusedSince).Round(time.Second).String(), Bound: readyRefusedFor.String(),
-				H2: "none: the merge is asked again after each verdict, Approve or composed verdict, and each minute — the operator's",
+				H2: "none: the merge is asked again on a verdict, an Approve, a group's composed check or a merge check's " +
+					"status turning success — the operator's",
 				Says: fmt.Sprintf("%s#%d is approved at %s, and the merge tool has refused it since %s: %s", d.Repository,
 					d.Number, shortOf(r.Head, 12), r.RefusedSince.UTC().Format(time.RFC3339), r.Answer)})
 		}

modules/mesh-delivery/cmd/mesh-delivery/ready_test.go

@@ -67,6 +67,8 @@ func TestAnApprovedPullRequestIsMergedThroughTheMergeTool(t *testing.T) {
 	}
 }
 
+// 1144/AC3: a refused merge is not asked again by any timer; only an event (here, mergeAgainNow called directly, as a
+// verdict, an Approve or a composed check does) asks it again.
 func TestAMergeRefusedIsSaidAtOnceAndAskedAgainOnlyOnAnEvent(t *testing.T) {
 	w := newWorld(t)
 	w.h.PullUpdated(pr("novox/app", 7, readyHead, "feat/x", "src/a.go"))
@@ -93,6 +95,88 @@ func TestAMergeRefusedIsSaidAtOnceAndAskedAgainOnlyOnAnEvent(t *testing.T) {
 	}
 }
 
+// 1144/AC1: a refused merge, approved and ready, is asked again at once when the forge's forge.status says one of
+// the merge checks (mesh/merge-gate, mesh/repo-check) is success for the commit the merge was refused at — the
+// delivery's approved head — and merges once the tool accepts (novox/hq mesh-catalog#366: the verdict reached this
+// owner before the forge wrote the statuses, the merge asked then was refused on a pending check, and nothing asked
+// it again when the statuses turned success).
+func TestAMergeRefusedIsAskedAgainWhenItsCheckTurnsSuccessOnTheForge(t *testing.T) {
+	w := newWorld(t)
+	w.h.PullUpdated(pr("novox/app", 7, readyHead, "feat/x", "src/a.go"))
+	w.h.Checked(judgedAt("novox/app", 7, readyHead, "pass", "t1t1t1t1t1t1"))
+	w.forge.refuse = errors.New("the forge refused gitea_merge_pull_request: mesh/repo-check (pending)")
+	w.forge.open = []OpenPull{openPull("novox/app", 7, readyHead)}
+	w.h.SeeOpenPulls()
+	w.settleAll()
+	if len(w.forge.merges) != 1 {
+		t.Fatalf("not asked once, refused: %v", w.forge.merges)
+	}
+	w.forge.refuse = nil
+	w.h.CommitStatus(ForgeStatusEvent{Owner: "novox", Repo: "app", SHA: readyHead, Context: "mesh/repo-check", State: "success"})
+	w.settleAll()
+	if len(w.forge.merges) != 2 {
+		t.Fatalf("not asked again when its check turned success: %v", w.forge.merges)
+	}
+	if w.delivery(IDOf("novox/app", readyHead)).Ready.Merged == nil {
+		t.Fatalf("it did not merge once the tool accepted")
+	}
+}
+
+// 1144/AC2: a status of another context, of another commit, or a state other than success changes nothing for a
+// refused merge: it is not asked again.
+func TestAMergeRefusedIsNotAskedAgainOnAStatusThatDoesNotMatch(t *testing.T) {
+	w := newWorld(t)
+	w.h.PullUpdated(pr("novox/app", 7, readyHead, "feat/x", "src/a.go"))
+	w.h.Checked(judgedAt("novox/app", 7, readyHead, "pass", "t1t1t1t1t1t1"))
+	w.forge.refuse = errors.New("the forge refused gitea_merge_pull_request: mesh/repo-check (pending)")
+	w.forge.open = []OpenPull{openPull("novox/app", 7, readyHead)}
+	w.h.SeeOpenPulls()
+	w.settleAll()
+	if len(w.forge.merges) != 1 {
+		t.Fatalf("not asked once, refused: %v", w.forge.merges)
+	}
+	w.forge.refuse = nil
+	for _, e := range []ForgeStatusEvent{
+		{Owner: "novox", Repo: "app", SHA: readyHead, Context: "some/other-check", State: "success"}, // another context
+		{Owner: "novox", Repo: "app", SHA: pushed, Context: "mesh/repo-check", State: "success"},     // another commit
+		{Owner: "novox", Repo: "app", SHA: readyHead, Context: "mesh/repo-check", State: "pending"},  // another state
+	} {
+		w.h.CommitStatus(e)
+		w.settleAll()
+	}
+	if len(w.forge.merges) != 1 {
+		t.Fatalf("asked again on a status that should change nothing: %v", w.forge.merges)
+	}
+	if w.delivery(IDOf("novox/app", readyHead)).Ready.Merged != nil {
+		t.Fatalf("merged on a status that should change nothing")
+	}
+}
+
+// 1144/AC4: the stalled line for a refused approved head names the events that ask the merge again, and no longer
+// says "each minute" (removed by issue 1115; false since then — there is no timer).
+func TestTheStalledLineForARefusedApprovedHeadNamesTheEventsThatAskTheMergeAgain(t *testing.T) {
+	w := newWorld(t)
+	w.h.PullUpdated(pr("novox/app", 7, readyHead, "feat/x", "src/a.go"))
+	w.h.Checked(judgedAt("novox/app", 7, readyHead, "pass", "t1t1t1t1t1t1"))
+	w.forge.refuse = errors.New("the forge refused gitea_merge_pull_request: mesh/repo-check (pending)")
+	w.forge.open = []OpenPull{openPull("novox/app", 7, readyHead)}
+	w.h.SeeOpenPulls()
+	w.settleAll()
+	lines := w.h.Stalled()
+	if len(lines) != 1 {
+		t.Fatalf("wanted one stalled line, got %+v", lines)
+	}
+	h2 := lines[0].H2
+	if strings.Contains(h2, "each minute") {
+		t.Fatalf("the stalled line still says 'each minute', false since issue 1115: %q", h2)
+	}
+	for _, want := range []string{"verdict", "Approve", "composed check", "status"} {
+		if !strings.Contains(h2, want) {
+			t.Fatalf("the stalled line does not name %q as asking the merge again: %q", want, h2)
+		}
+	}
+}
+
 func TestAGroupMergesEveryMemberInOnePassOnlyOnceComposedAndApproved(t *testing.T) {
 	w := newWorld(t)
 	w.op.auto = false

modules/mesh-delivery/cmd/mesh-delivery/timers_test.go

@@ -55,6 +55,7 @@ func TestAProposedDeliveryWhoseHeadHoldsAPassingVerdictAtTheForgeAdoptsItAndAsks
 	}
 }
 
+// 1144/AC3: no timer asks a refused merge again; only an event does (here, mergeAgainNow called directly).
 func TestAMergeTheForgeRefusedIsAskedAgainOnlyOnAnEventWithNoTimer(t *testing.T) {
 	w := newWorld(t)
 	w.h.PullUpdated(pr("novox/app", 7, readyHead, "feat/x", "src/a.go"))

modules/mesh-issues/cmd/mesh-issues/manifest_test.go

@@ -159,7 +159,8 @@ func TestTheSettingsStartTheCounterAndRegisterTheOwners(t *testing.T) {
 	if !sort.StringsAreSorted(s.Owners) {
 		t.Error("the owners are not sorted")
 	}
-	for _, repo := range []string{"hq", "mesh-catalog", "mesh-controller", "mesh-host", "mesh-sdk", "mesh-tools"} {
+	// 1133/AC1: the repository mesh-delivery is registered, so an issue can be located in it.
+	for _, repo := range []string{"hq", "mesh-catalog", "mesh-controller", "mesh-delivery", "mesh-host", "mesh-sdk", "mesh-tools"} {
 		if Owners(s.Owners).owns(repo) != repo {
 			t.Errorf("%s is not registered", repo)
 		}
@@ -178,3 +179,34 @@ func TestTheSettingsStartTheCounterAndRegisterTheOwners(t *testing.T) {
 		}
 	}
 }
+
+// The owners the tracker runs with are the manifest's default (no layer sets them), and the template's copy is
+// what the test above reads: the two are one list, or that test proves a list nobody runs (novox/hq issue 1133).
+// 1133/AC2
+func TestTheOwnersDefaultIsTheTemplatesOwners(t *testing.T) {
+	raw, err := os.ReadFile(filepath.Join("..", "..", "module.json"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	var m struct {
+		Config struct {
+			Owners struct {
+				Default []string `json:"default"`
+			} `json:"owners"`
+		} `json:"config"`
+	}
+	if err := json.Unmarshal(raw, &m); err != nil {
+		t.Fatal(err)
+	}
+	tmpl, err := os.ReadFile(filepath.Join("..", "..", "files", "settings.json"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	var s Settings
+	if err := json.Unmarshal(tmpl, &s); err != nil {
+		t.Fatal(err)
+	}
+	if len(m.Config.Owners.Default) == 0 || !reflect.DeepEqual(m.Config.Owners.Default, s.Owners) {
+		t.Fatalf("the owners default (%d) and the template's owners (%d) differ", len(m.Config.Owners.Default), len(s.Owners))
+	}
+}

modules/mesh-issues/files/settings.json

@@ -128,6 +128,7 @@
     "mesh-controller",
     "mesh-controller modules/build-agent",
     "mesh-controller modules/route-proxy",
+    "mesh-delivery",
     "mesh-host",
     "mesh-lab",
     "mesh-media-catalog",

modules/mesh-issues/module.json

@@ -221,6 +221,7 @@
         "mesh-controller",
         "mesh-controller modules/build-agent",
         "mesh-controller modules/route-proxy",
+        "mesh-delivery",
         "mesh-host",
         "mesh-lab",
         "mesh-media-catalog",