Merge pull request 'mesh-delivery announces each merge with the Approve it merged on (issue 1025, ADR 0420)' (#348) from feat/move-a-module into main

2026-10-17 16:00:29 +00:00
parent 0d2fc5e4d7 40eed3a036
commit 54b3ce503e
6 changed files with 154 additions and 2 deletions
@@ -251,3 +251,130 @@ func TestAPushToAnUnmarkedGroupMemberEndsTheGroupsAskAndApprove(t *testing.T) {
 		t.Fatalf("merged on an Approve of heads the group no longer has: %v", w.forge.merges)
 	}
 }
+
+// mergedSaid is the module events `merged` the world has said.
+func mergedSaid(w *world) []map[string]any {
+	w.mu.Lock()
+	defer w.mu.Unlock()
+	var out []map[string]any
+	for _, m := range w.said {
+		if m["event-kind"] == "merged" {
+			out = append(out, m)
+		}
+	}
+	return out
+}
+
+// Issue 1025: a merge on the operator's Approve is announced once, with the warrant's ask, option and heads.
+func TestIssue1025AMergeOnTheApproveIsAnnouncedWithItsWarrant(t *testing.T) {
+	w := markedGreen(t)
+	id := IDOf("novox/app", readyHead)
+	open := w.op.open()
+	if len(open) != 1 {
+		t.Fatalf("asks %v", open)
+	}
+	if got := mergedSaid(w); len(got) != 0 {
+		t.Fatalf("announced before the merge: %v", got)
+	}
+	d := w.delivery(id)
+	wantHeads := map[string]string{}
+	for k, v := range d.Ready.Heads {
+		wantHeads[k] = v
+	}
+	w.op.answer(open[0], OptionApprove)
+	w.settleAll()
+	got := mergedSaid(w)
+	if len(got) != 1 {
+		t.Fatalf("announced %d times; wanted once: %v", len(got), got)
+	}
+	m := got[0]
+	if m["owner"] != "novox" || m["repo"] != "app" || m["number"] != float64(7) || m["head_sha"] != readyHead ||
+		m["ask_id"] != open[0] || m["option"] != OptionApprove {
+		t.Fatalf("the event does not name the pull request and its warrant: %v", m)
+	}
+	hs, _ := m["heads"].(map[string]any)
+	if len(hs) == 0 || len(hs) != len(wantHeads) {
+		t.Fatalf("heads %v; wanted %v", hs, wantHeads)
+	}
+	for k, v := range wantHeads {
+		if hs[k] != v {
+			t.Fatalf("heads %v; wanted %v", hs, wantHeads)
+		}
+	}
+	if g, ok := m["group"]; ok && g != "" {
+		t.Fatalf("a lone delivery names group %v", g)
+	}
+	w.later(2 * time.Minute)
+	w.settleAll()
+	if n := len(mergedSaid(w)); n != 1 {
+		t.Fatalf("announced %d times after more ticks", n)
+	}
+}
+
+// Issue 1025: a declined merge announces nothing.
+func TestIssue1025ADeclinedMergeAnnouncesNothing(t *testing.T) {
+	w := markedGreen(t)
+	open := w.op.open()
+	w.op.answer(open[0], OptionDecline)
+	w.settleAll()
+	if got := mergedSaid(w); len(got) != 0 || len(w.forge.merges) != 0 {
+		t.Fatalf("declined, yet merges %v, announced %v", w.forge.merges, got)
+	}
+}
+
+// Issue 1025: a merge of a delivery in a group carries the group id and the heads the group's Approve was bound to.
+func TestIssue1025AMergeInAGroupCarriesTheGroupAndItsHeads(t *testing.T) {
+	w := newWorld(t)
+	w.op.auto = false
+	const ctlHead, catHead = "c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1", "d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2"
+	ctl, cat := IDOf("novox/mesh-controller", ctlHead), IDOf("novox/mesh-catalog", catHead)
+	w.ctl.order = func(ms []Member) Order { return Order{Order: []string{ctl, cat}} }
+	w.h.PullUpdated(pr("novox/mesh-controller", 1, ctlHead, "feat/x", "internal/a.go"))
+	w.h.PullUpdated(pr("novox/mesh-catalog", 2, catHead, "feat/x", "modules/app/module.json"))
+	w.h.Checked(judgedAt("novox/mesh-controller", 1, ctlHead, "pass", "t1t1t1t1t1t1"))
+	w.h.Checked(judgedAt("novox/mesh-catalog", 2, catHead, "pass", "t3t3t3t3t3t3"))
+	w.forge.open = []OpenPull{markedPull("novox/mesh-controller", 1, ctlHead, ctlHead),
+		markedPull("novox/mesh-catalog", 2, catHead, catHead)}
+	w.h.SeeOpenPulls()
+	w.settleAll()
+	w.h.Checked(CheckedEvent{Group: "feat/x", ID: "check-1", Verdict: "pass", Summary: "both compose"})
+	w.settleAll()
+	open := w.op.open()
+	if len(open) != 1 {
+		t.Fatalf("asks %v", open)
+	}
+	w.op.answer(open[0], OptionApprove)
+	w.settleAll()
+	got := mergedSaid(w)
+	if len(got) != 1 {
+		t.Fatalf("announced %d; wanted the first member's merge once: %v", len(got), got)
+	}
+	m := got[0]
+	hs, _ := m["heads"].(map[string]any)
+	if m["group"] != "feat/x" || m["ask_id"] != open[0] || m["option"] != OptionApprove || m["repo"] != "mesh-controller" ||
+		hs[ctl] != ctlHead || hs[cat] != catHead || len(hs) != 2 {
+		t.Fatalf("the group's merge event: %v", m)
+	}
+}
+
+// Issue 1025: the announcement is owed and kept, so it is said after a restart, and once.
+func TestIssue1025AnOwedMergedIsSaidAfterARestartAndOnce(t *testing.T) {
+	w := newWorld(t)
+	p := pr("novox/app", 7, readyHead, "feat/x", "src/a.go")
+	w.h.PullUpdated(p)
+	id := IDOf("novox/app", readyHead)
+	w.h.mu.Lock()
+	d := w.h.deliveries[id]
+	d.Owed = append(d.Owed, Effect{Kind: EffectEmit, Event: "merged", Line: `{"owner":"novox","repo":"app","number":7,"head_sha":"` + readyHead + `"}`, Since: w.now})
+	if err := w.store.PutDelivery(d); err != nil {
+		t.Fatal(err)
+	}
+	w.h.mu.Unlock()
+	again := newWorldOver(t, w.store, w.ctl, w.forge, w.now.Add(time.Minute))
+	again.settleAll()
+	again.later(time.Hour)
+	again.settleAll()
+	if n := len(mergedSaid(again)); n != 1 {
+		t.Fatalf("said %d times after the restart; wanted once", n)
+	}
+}
@@ -428,6 +428,9 @@ func (h *Holder) do(e Effect, repo [2]string, number int, body, url string, grou
 		if group {
 			event = "group"
 		}
+		if e.Event != "" {
+			event = e.Event
+		}
 		return h.Emit(event, payload)
 	case EffectNote:
 		if h.Forge == nil {
@@ -54,7 +54,7 @@ func TestItClaimsTheDeliverySeatAndFollowsWhatItHandles(t *testing.T) {
 	if !reflect.DeepEqual(m.Consumes, followed) {
 		t.Fatalf("consumes %v, follows %v", m.Consumes, followed)
 	}
-	if !reflect.DeepEqual(m.Emits, []string{"transition", "group"}) {
+	if !reflect.DeepEqual(m.Emits, []string{"transition", "group", "merged"}) {
 		t.Fatalf("emits %v", m.Emits)
 	}
 	if !reflect.DeepEqual(m.State, []string{"deliveries", "groups", "asked", "pulls"}) {
@@ -282,6 +282,7 @@ type Step struct {
 // on the pull request refreshed, a status set.
 type Effect struct {
 	Kind    string    `json:"kind"`
+	Event   string    `json:"event,omitempty"` // an emit's module event, when not transition or group (issue 1025)
 	Commit  string    `json:"commit,omitempty"`
 	Line    string    `json:"line,omitempty"`
 	Context string    `json:"context,omitempty"`
@@ -351,6 +351,9 @@ func (h *Holder) askMerge(id string, p OpenPull, head string) func() {
 		r, now := d.Ready, h.Now()
 		r.merging = false
 		if err == nil {
+			// Issue 1025: the announcement is owed, kept with the delivery, so a death right after the merge
+			// still says it once.
+			d.Owed = append(d.Owed, Effect{Kind: EffectEmit, Event: "merged", Line: string(mustJSON(mergedEvent(d, p, head))), Since: now})
 			r.Merged, r.Answer, r.RefusedSince = &now, "merged", nil
 			h.Logf("[mesh-delivery] %s merged on its ready mark at %s, its check passed against its trunk's head", id,
 				shortOf(head, 12))
@@ -467,3 +470,20 @@ func (h *Holder) readyLines(now time.Time) []StalledLine {
 	}
 	return out
 }
+
+// mergedEvent is the module event `merged` (issue 1025): the pull request merged, its head and group, and the ask,
+// option and heads the operator's Approve was bound to (the warrant the controller reads back from the router). A merge
+// without an Approve names no ask and no option: no warrant is invented. Under the lock, before anything clears them.
+func mergedEvent(d *Delivery, p OpenPull, head string) map[string]any {
+	r := d.Ready
+	heads := map[string]string{}
+	for k, v := range r.Heads {
+		heads[k] = v
+	}
+	ev := map[string]any{"owner": p.Owner, "repo": p.Repo, "number": p.Number, "head_sha": head, "group": d.Group,
+		"ask_id": "", "option": "", "heads": heads}
+	if r.Approved != nil {
+		ev["ask_id"], ev["option"] = r.AskID, OptionApprove
+	}
+	return ev
+}
@@ -37,7 +37,8 @@
  ],
  "emits": [
    "transition",
-    "group"
+    "group",
+    "merged"
  ],
  "invokes": [
    "seat:mesh-controller.delivery-plan",