From 7383bc31703eca05b6663e351f844c6568da2d18 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 18 Oct 2026 01:20:18 +0200 Subject: [PATCH] A stale composed pass counts members judged since it was asked, whichever verdict lands first (issue 1047, review round 2) --- .../cmd/mesh-delivery/group_test.go | 37 +++++++++++++++++++ .../mesh-delivery/cmd/mesh-delivery/holder.go | 4 +- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/modules/mesh-delivery/cmd/mesh-delivery/group_test.go b/modules/mesh-delivery/cmd/mesh-delivery/group_test.go index faa9dbf4..88967366 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/group_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/group_test.go @@ -358,3 +358,40 @@ func TestATrunkMoveAsksTheCheckOfAGroupMemberWhoseApproveIsNotAskedYet(t *testin t.Fatalf("the operator is asked once, on the new pass: %d", w.op.count()) } } + +// Issue 1047 (review round 2): the other order. The moved member's fresh verdict lands before the stale composed pass (one +// build against two): the pass is out of date as it lands, the composed check is asked again, and the operator is asked +// only after that check passes. +func TestAStaleComposedPassLandingAfterTheMovedMembersFreshVerdictIsAskedAgain(t *testing.T) { + w := newWorld(t) + w.op.auto = false + 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")) + w.h.Checked(judged(gCtl, 1, gHc, "pass", gTrunk0)) + w.h.Checked(judged(gCat, 2, gHk, "pass", gTrunk0)) + w.settleAll() + g := w.h.groups["feat/g"] + w.later(time.Minute) + w.h.PullMerged(merged(gCat, 5, "5555555555555555", gT1)) + w.settleAll() + w.later(time.Minute) + w.h.Checked(judged(gCat, 2, gHk, "pass", gT1)) // the fresh verdict first + w.settleAll() + w.later(time.Minute) + before := len(w.ctl.checks) + w.h.Checked(CheckedEvent{Group: "feat/g", ID: g.Check.Asked, Verdict: "pass", Summary: "composed on the old trunk", + Members: []struct { + Owner string `json:"owner"` + Repo string `json:"repo"` + Commit string `json:"commit"` + Trunk string `json:"trunk,omitempty"` + }{{Owner: "novox", Repo: "mesh-controller", Commit: gHc, Trunk: gTrunk0[:12]}, + {Owner: "novox", Repo: "mesh-catalog", Commit: gHk, Trunk: gTrunk0[:12]}}}) + w.settleAll() + if len(w.ctl.checks) == before || !strings.HasPrefix(w.ctl.checks[len(w.ctl.checks)-1], "feat/g:") { + t.Fatalf("the composed check was not asked again after the stale pass: %v", w.ctl.checks[before:]) + } + if w.op.count() != 0 { + t.Fatalf("the operator was asked before the new composed check passed: %d", w.op.count()) + } +} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/holder.go b/modules/mesh-delivery/cmd/mesh-delivery/holder.go index 77f7013b..b8fa3fcf 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/holder.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/holder.go @@ -836,7 +836,9 @@ func (h *Holder) Checked(c CheckedEvent) { if d := h.deliveries[id]; d != nil && d.MergedAs == "" && !d.State.Final() && d.TrunkNow != "" && !sameCommit(judged, d.TrunkNow) { g.Check.OutOfDate = fmt.Sprintf("composed check out of date: %s was judged against trunk %s, the trunk is now %s", id, shortOf(judged, 12), shortOf(d.TrunkNow, 12)) - g.Check.OutOfDateAt = now + // From when the composed check was asked: a member judged since then, even before this verdict landed, + // counts as judged again. + g.Check.OutOfDateAt = g.Check.At h.Logf("[mesh-delivery] group %s: %s", g.ID, g.Check.OutOfDate) break }