From 4c0ed75090f25b4b6f89822d4d2060471e26a0c9 Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 17 Oct 2026 15:40:00 +0200 Subject: [PATCH 1/2] issue 1025: mesh-delivery announces each merge on the Approve as the event merged, with its warrant --- .../cmd/mesh-delivery/approval_test.go | 70 +++++++++++++++++++ .../cmd/mesh-delivery/manifest_test.go | 2 +- .../mesh-delivery/cmd/mesh-delivery/ready.go | 28 +++++++- modules/mesh-delivery/module.json | 3 +- 4 files changed, 100 insertions(+), 3 deletions(-) diff --git a/modules/mesh-delivery/cmd/mesh-delivery/approval_test.go b/modules/mesh-delivery/cmd/mesh-delivery/approval_test.go index aa1454b4..56ec5061 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/approval_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/approval_test.go @@ -251,3 +251,73 @@ 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) + } +} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/manifest_test.go b/modules/mesh-delivery/cmd/mesh-delivery/manifest_test.go index b724779a..7093e435 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/manifest_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/manifest_test.go @@ -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"}) { diff --git a/modules/mesh-delivery/cmd/mesh-delivery/ready.go b/modules/mesh-delivery/cmd/mesh-delivery/ready.go index 32c23bd0..c1075bee 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/ready.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/ready.go @@ -343,7 +343,15 @@ func (h *Holder) askMerge(id string, p OpenPull, head string) func() { return func() { err := h.Forge.Merge(p.Owner, p.Repo, p.Number, head) h.mu.Lock() - defer h.mu.Unlock() + var announce map[string]any // issue 1025: said after unlocking + defer func() { + h.mu.Unlock() + if announce != nil { + if e := h.Emit("merged", announce); e != nil { + h.Logf("[mesh-delivery] could not announce the merge of %s: %v", id, e) + } + } + }() d := h.deliveries[id] if d == nil || d.Ready == nil { return @@ -351,6 +359,7 @@ func (h *Holder) askMerge(id string, p OpenPull, head string) func() { r, now := d.Ready, h.Now() r.merging = false if err == nil { + announce = mergedEvent(d, p, head) 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 +476,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 +} diff --git a/modules/mesh-delivery/module.json b/modules/mesh-delivery/module.json index 8cad5f5e..80f1cc49 100644 --- a/modules/mesh-delivery/module.json +++ b/modules/mesh-delivery/module.json @@ -37,7 +37,8 @@ ], "emits": [ "transition", - "group" + "group", + "merged" ], "invokes": [ "seat:mesh-controller.delivery-plan", -- 2.54.0 From 3ec2881a27f870121ef22f831b5279097e90fda2 Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 17 Oct 2026 15:41:43 +0200 Subject: [PATCH 2/2] issue 1025: the merged announcement is an owed effect, and a group's merge is tested --- .../cmd/mesh-delivery/approval_test.go | 57 +++++++++++++++++++ .../cmd/mesh-delivery/effects.go | 3 + .../mesh-delivery/cmd/mesh-delivery/model.go | 1 + .../mesh-delivery/cmd/mesh-delivery/ready.go | 14 ++--- 4 files changed, 65 insertions(+), 10 deletions(-) diff --git a/modules/mesh-delivery/cmd/mesh-delivery/approval_test.go b/modules/mesh-delivery/cmd/mesh-delivery/approval_test.go index 56ec5061..95f2b2cb 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/approval_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/approval_test.go @@ -321,3 +321,60 @@ func TestIssue1025ADeclinedMergeAnnouncesNothing(t *testing.T) { 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) + } +} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/effects.go b/modules/mesh-delivery/cmd/mesh-delivery/effects.go index 3c20d918..b74ae890 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/effects.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/effects.go @@ -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 { diff --git a/modules/mesh-delivery/cmd/mesh-delivery/model.go b/modules/mesh-delivery/cmd/mesh-delivery/model.go index d4b4f68e..ef6631b1 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/model.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/model.go @@ -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"` diff --git a/modules/mesh-delivery/cmd/mesh-delivery/ready.go b/modules/mesh-delivery/cmd/mesh-delivery/ready.go index c1075bee..9a67ef3b 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/ready.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/ready.go @@ -343,15 +343,7 @@ func (h *Holder) askMerge(id string, p OpenPull, head string) func() { return func() { err := h.Forge.Merge(p.Owner, p.Repo, p.Number, head) h.mu.Lock() - var announce map[string]any // issue 1025: said after unlocking - defer func() { - h.mu.Unlock() - if announce != nil { - if e := h.Emit("merged", announce); e != nil { - h.Logf("[mesh-delivery] could not announce the merge of %s: %v", id, e) - } - } - }() + defer h.mu.Unlock() d := h.deliveries[id] if d == nil || d.Ready == nil { return @@ -359,7 +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 { - announce = mergedEvent(d, p, head) + // 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)) -- 2.54.0