From bc1acddaa4e55edac89a61e3df53945a2c67e228 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 18 Oct 2026 04:55:48 +0200 Subject: [PATCH] mesh-delivery hears the forge's own pull-request closed event as a close unmerged (issue 1107) --- .../mesh-delivery/cmd/mesh-delivery/holder.go | 3 + .../cmd/mesh-delivery/issue1107_test.go | 61 +++++++++++++++++++ .../mesh-delivery/cmd/mesh-delivery/main.go | 13 +++- modules/mesh-delivery/module.json | 1 + 4 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 modules/mesh-delivery/cmd/mesh-delivery/issue1107_test.go diff --git a/modules/mesh-delivery/cmd/mesh-delivery/holder.go b/modules/mesh-delivery/cmd/mesh-delivery/holder.go index d9a98464..78069b17 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/holder.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/holder.go @@ -310,6 +310,9 @@ type PullEvent struct { HeadChecks map[string]string `json:"head_checks,omitempty"` // BaseSHA is the base branch's head when the head was announced (novox/hq ADR 0323). BaseSHA string `json:"base_sha,omitempty"` + // Merged is the forge's own events' word that the pull request merged (gitea.forge.pull-request.*): its close is no + // close unmerged (issue 1107). + Merged bool `json:"merged,omitempty"` } func (p PullEvent) repository() string { return strings.ToLower(p.Owner + "/" + p.Repo) } diff --git a/modules/mesh-delivery/cmd/mesh-delivery/issue1107_test.go b/modules/mesh-delivery/cmd/mesh-delivery/issue1107_test.go new file mode 100644 index 00000000..f9dea49d --- /dev/null +++ b/modules/mesh-delivery/cmd/mesh-delivery/issue1107_test.go @@ -0,0 +1,61 @@ +package main + +import ( + "encoding/json" + "testing" +) + +// Issue 1107: a pull request closed unmerged in a quiet repository is heard from the forge's own event +// (gitea.forge.pull-request.closed), not only from the poll, which skips a repository whose update time did not move. +// Its delivery leaves its group and stops; a merged one is ignored (the webhook says merged for it); a close heard +// twice, from the event and the poll, changes nothing the second time. +func forgeClosed(owner, repo string, number int, merged bool) map[string]any { + return map[string]any{"owner": owner, "repo": repo, "number": number, "merged": merged} +} + +func TestAForgeClosedEventTakesAGroupMembersDeliveryOutOfItsGroupAndStopsIt(t *testing.T) { + w := newWorld(t) + w.h.PullUpdated(pr(gCtl, 1, gHc, "feat/g", "internal/a.go")) + w.h.PullUpdated(pr(gCat, 2, gHk, "feat/g", "modules/app/module.json")) + id := IDOf(gCtl, gHc) + hear(t, w, "gitea.forge.pull-request.closed", forgeClosed("novox", "mesh-controller", 1, false)) + d := w.delivery(id) + if !d.ClosedUnmerged || !d.State.Final() || inGroup(w, "feat/g", id) { + t.Fatalf("closed %v, state %s, in its group %v; wanted closed, stopped, out of its group", d.ClosedUnmerged, d.State, + inGroup(w, "feat/g", id)) + } + // Heard again, from the event and from the poll: nothing changes. + before, _ := json.Marshal(d) + hear(t, w, "gitea.forge.pull-request.closed", forgeClosed("novox", "mesh-controller", 1, false)) + hear(t, w, "gitea.pull.closed", map[string]any{"owner": "novox", "repo": "mesh-controller", "number": 1}) + if after, _ := json.Marshal(w.delivery(id)); string(after) != string(before) { + t.Fatalf("a second close changed it:\n%s\n%s", before, after) + } +} + +func TestAForgeClosedEventOfAMergedPullRequestIsIgnored(t *testing.T) { + w := newWorld(t) + w.h.PullUpdated(pr(gCtl, 1, gHc, "feat/g", "internal/a.go")) + w.h.PullUpdated(pr(gCat, 2, gHk, "feat/g", "modules/app/module.json")) + id := IDOf(gCtl, gHc) + hear(t, w, "gitea.forge.pull-request.closed", forgeClosed("novox", "mesh-controller", 1, true)) + if d := w.delivery(id); d.ClosedUnmerged || !inGroup(w, "feat/g", id) || d.State.Final() { + t.Fatalf("a merged pull request's close stopped it: closed %v, %s", d.ClosedUnmerged, d.State) + } +} + +// inGroup is whether the group still lists the delivery as a member. +func inGroup(w *world, group, id string) bool { + w.h.mu.Lock() + defer w.h.mu.Unlock() + g := w.h.groups[group] + if g == nil { + return false + } + for _, m := range g.Members { + if m == id { + return true + } + } + return false +} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/main.go b/modules/mesh-delivery/cmd/mesh-delivery/main.go index 68bb0f56..b61d5158 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/main.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/main.go @@ -34,7 +34,7 @@ func (l *listening) get() string { l.mu.Lock(); defer l.mu.Unlock(); return l.n // The events this owner follows: the forge's pull requests, and the controller's verdicts and walks. // The forge's status events say that a check answered (events.go). var followed = []string{"gitea.pull.updated", "gitea.pull.merged", "gitea.pull.closed", - "gitea.forge.status", "gitea.forge.pull-request.edited", ControllerSeat + ".checked", ControllerSeat + ".plan-moved"} + "gitea.forge.status", "gitea.forge.pull-request.edited", "gitea.forge.pull-request.closed", ControllerSeat + ".checked", ControllerSeat + ".plan-moved"} func main() { operator := &asker.Client{Module: Module, State: stdio.State("asked")} @@ -157,6 +157,17 @@ func take(h *Holder, e stdio.Envelope) error { return nil } h.PullEdited(p) + case "gitea.forge.pull-request.closed": + // The forge's own close event (issue 1107): the poll's gitea.pull.closed skips a repository whose update time did not + // move, and a close does not move it. A merged pull request is the webhook's merged event, never this one. + var p PullEvent + if err := json.Unmarshal(e.Body, &p); err != nil { + h.Refuse(fmt.Errorf("%s is not readable: %v", e.Key, err)) + return nil + } + if !p.Merged { + h.PullClosed(p) + } case "gitea.forge.status": var s ForgeStatusEvent if err := json.Unmarshal(e.Body, &s); err != nil { diff --git a/modules/mesh-delivery/module.json b/modules/mesh-delivery/module.json index 5f2f180d..1a40896b 100644 --- a/modules/mesh-delivery/module.json +++ b/modules/mesh-delivery/module.json @@ -29,6 +29,7 @@ "gitea.pull.closed", "gitea.forge.status", "gitea.forge.pull-request.edited", + "gitea.forge.pull-request.closed", "mesh-controller.checked", "mesh-controller.plan-moved" ],