From 40da368ee34e0828b417473a0bf82139d48388c0 Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 24 Oct 2026 13:55:10 +0200 Subject: [PATCH] Date an adopted merge once, by the walk it was adopted from, so a later walk it follows never makes it look newer than a forge merge heard after it (issue 1726, review) --- .../cmd/mesh-delivery-gate/holder.go | 6 ++++ .../issue1726_trunk_test.go | 35 +++++++++++++++++++ .../cmd/mesh-delivery-gate/merges.go | 14 +++----- 3 files changed, 46 insertions(+), 9 deletions(-) diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go index 2fb5dc84..70c37dbf 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go @@ -1848,6 +1848,12 @@ func (h *Holder) adopt(w Walk) { now := h.Now() d := &Delivery{ID: IDOf(w.Repository, w.Commit), Repository: strings.ToLower(w.Repository), Commit: w.Commit, Base: w.Branch, MergedAs: w.Commit, Created: now} + // Dated once, by the walk it is adopted from, which follows the merge: a later walk it follows (linkWalk) never redates + // it, so it never looks newer than a merge heard after it (issue 1726, review). + d.MergedAt = w.Created.UTC() + if w.Created.IsZero() { + d.MergedAt = now + } if old := h.deliveries[d.ID]; old != nil { if old.State.Final() { // Never revived: an ended delivery is not the walk's; the walk, held by nothing, is listed by diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1726_trunk_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1726_trunk_test.go index 3b8ee33e..c7f89b4d 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1726_trunk_test.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1726_trunk_test.go @@ -124,3 +124,38 @@ func TestAnEventBeforeTheGateKnowsWhetherItHoldsTheSeatIsOfferedAgain(t *testing t.Fatalf("an event heard while today's module holds the seat was offered again: %v", err) } } + +// 1726c/AC4 (review round 1): an adopted merge that later follows a newer walk keeps the date of the walk it was adopted +// from, so a merge heard from the forge after it stays in source.main through a catch-up. +func TestAnAdoptedMergeFollowingANewerWalkNeverMovesSourceMainBack(t *testing.T) { + w := newWorld(t) + walk := aWalk("plan-471", i1726Repo, i1726Merge2, false) + walk.Created = w.now.Add(-10 * time.Minute) + walk.State = walkRolling + w.ctl.put(walk) + if err := w.h.Reconcile(); err != nil { + t.Fatal(err) + } + w.settleAll() + if got := w.source.main.Heads[i1726Repo]; got != i1726Merge2 { + t.Fatalf("the adopted merge did not move source.main: %s", got) + } + const later = "eeee000000000000000000000000000000000004" + w.h.PullUpdated(pr(i1726Repo, 473, "ffff000000000000000000000000000000000001", "fix/c", "modules/app/c.go")) + w.h.PullMerged(i888MergedAt(i1726Repo, 473, "ffff000000000000000000000000000000000001", later, w.now.Add(-5*time.Minute))) + w.settleAll() + if got := w.source.main.Heads[i1726Repo]; got != later { + t.Fatalf("the later forge merge did not move source.main: %s", got) + } + again := aWalk("plan-471-again", i1726Repo, i1726Merge2, false) + again.Created = w.now.Add(-time.Minute) + again.State = walkRolling + w.h.mu.Lock() + w.h.linkWalk(w.h.deliveries[IDOf(i1726Repo, i1726Merge2)], again, i1726Merge2) + w.h.mu.Unlock() + w.h.SourceCatchUp() + w.settleAll() + if got := w.source.main.Heads[i1726Repo]; got != later { + t.Fatalf("the catch-up moved source.main back to the adopted merge %s after it followed a newer walk", got) + } +} diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/merges.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/merges.go index 157b7624..2e2ff9e3 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/merges.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/merges.go @@ -435,7 +435,7 @@ func gateMerges(d *Delivery) bool { // happens outside it. // // A merge caught up after a gap (issue 1726) is settled the same way: the controller's walk of a merge commit whose -// pull.merged the gate never heard is adopted with the walk's time as its merge's (mergedWhen), and once each time the +// pull.merged the gate never heard is adopted with the walk's time as its merge's (adopt, mergedWhen), and once each time the // gate comes to hold the seat (SourceCatchUp) the newest merge of each repository it holds is weighed again though it is // marked in the source, so a merge marked there and never written reaches source.main, and with it the checker. func (h *Holder) settleForgeMerges() []func() { @@ -529,16 +529,12 @@ func (h *Holder) olderHead(repository, base, head string, at time.Time) bool { return at.After(h.Now().Add(-keptFinal)) } -// mergedWhen is when a merged delivery's merge landed: the forge's time, or for a merge the gate knows only from the -// controller's walk of its merge commit (adopt: no pull request heard, so no forge time) the walk's creation, which follows -// the merge; else when the gate made the delivery. A zero time made every other merge of the repository look newer, so an -// adopted merge never moved source.main (issue 1726). +// mergedWhen is when a merged delivery's merge landed: its merge time (the forge's, or for a merge adopted from the +// controller's walk the walk's creation, set once in adopt), else when the gate made the delivery. A zero time made every +// other merge of the repository look newer, so an adopted merge never moved source.main (issue 1726). func mergedWhen(d *Delivery) time.Time { - switch { - case !d.MergedAt.IsZero(): + if !d.MergedAt.IsZero() { return d.MergedAt - case d.Walk != nil && !d.Walk.Created.IsZero(): - return d.Walk.Created } return d.Created }