From dfcdf234c564ce0cba991a9e48a9a4c8e592db1b Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 23 Oct 2026 12:36:58 +0200 Subject: [PATCH] Never move source.main back for a forge-heard merge, by dating the head it holds (issue 888, review round 1) A merge the gate handed is known by the bridge's word before the forge's, and finish wrote it already; an older forge-only merge heard in between overwrote it, since only the forge's word was dated and the write never compared with the head held. A merge is now dated by either word, and the write leaves a head that is a later merge, or one the gate cannot date while the merge is older than the records it keeps (a pruned newer merge). --- .../cmd/mesh-delivery-gate/issue888_test.go | 31 +++++++++++++++ .../cmd/mesh-delivery-gate/merges.go | 39 +++++++++++++++++-- 2 files changed, 66 insertions(+), 4 deletions(-) diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue888_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue888_test.go index 54d777e0..725029ff 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue888_test.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue888_test.go @@ -101,3 +101,34 @@ func TestAnApprovedGroupMergedAtTheForgeMovesSourceMainOnce(t *testing.T) { t.Fatalf("source.main moved %d times to %+v; wanted once, both members at their merge commits", len(w.source.moves), w.source.main) } } + +// 888/AC5 (review round 1): an older merge heard from the forge after the bridge merged a newer one, whose own pull.merged is +// not heard yet, never moves source.main back; nor does one older than the gate keeps records for, over a head it does not know. +func TestAnOlderForgeMergeNeverOverwritesANewerBridgeMergeOrAnUnknownHead(t *testing.T) { + w := approvedGroup(t) + allGreen(w) + w.op.answer(w.op.open()[0], OptionApprove) + w.settleAll() + w.settleAll() + if len(w.source.moves) != 1 { + t.Fatalf("the group did not move source.main once: %d", len(w.source.moves)) + } + bridged := w.source.main.Heads[gCat] + w.h.PullUpdated(pr(gCat, 7, "7777777777777777777777777777777777777777", "fix/old", "modules/app/module.json")) + w.h.PullMerged(i888MergedAt(gCat, 7, "7777777777777777777777777777777777777777", "8888888888888888888888888888888888888888", w.now.Add(-10*time.Minute))) + w.settleAll() + if w.source.main.Heads[gCat] != bridged || len(w.source.moves) != 1 { + t.Fatalf("an older forge merge moved source.main from the bridge's %s to %s", bridged, w.source.main.Heads[gCat]) + } + + o := newWorld(t) + o.source.main = &sdk.Source{V: sdk.Version, Group: "pruned", Heads: map[string]string{i888App: i888Merge2}} + at := o.now + o.h.PullUpdated(pr(i888App, 3, i888Head1, "fix/x", "internal/a.go")) + o.later(keptFinal + 24*time.Hour) + o.h.PullMerged(i888MergedAt(i888App, 3, i888Head1, i888Merge1, at.Add(time.Minute))) + o.settleAll() + if o.source.main.Heads[i888App] != i888Merge2 || len(o.source.moves) != 0 { + t.Fatalf("a merge older than the records kept overwrote a head the gate does not know: %+v", o.source.main) + } +} 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 c8a5abaf..01692ce6 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/merges.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/merges.go @@ -444,14 +444,18 @@ func (h *Holder) settleForgeMerges() []func() { h.keep(d) continue } - id, repository, commit, group := d.ID, d.Repository, d.MergedAs, firstOf(d.Branch, d.Repository) + id, repository, base, commit, at, group := d.ID, d.Repository, d.Base, d.MergedAs, d.MergedAt, firstOf(d.Branch, d.Repository) out = append(out, func() { err := h.Source.Move(func(prev *sdk.Source) (sdk.Source, bool) { next := sdk.Source{Group: group, Heads: map[string]string{repository: commit}} if prev != nil { - if sameCommit(prev.Heads[repository], commit) { + held := prev.Heads[repository] + if sameCommit(held, commit) { return next, false // there already: the bridge's word and the forge's are one merge } + if held != "" && !h.olderHead(repository, base, held, at) { + return next, false // source.main holds a newer merge, or one the gate cannot date: never moved back + } for r, head := range prev.Heads { if r != repository { next.Heads[r] = head @@ -477,13 +481,40 @@ func (h *Holder) settleForgeMerges() []func() { return out } +// mergeOf is a delivery's merge into its trunk as the gate knows it: the forge's word (MergedAs), or else the bridge's +// (Ready.MergeCommit, Ready.Merged), which comes first for a merge the gate handed. Empty when it is not merged. +func mergeOf(d *Delivery) (string, time.Time) { + if d.MergedAs != "" { + return d.MergedAs, d.MergedAt + } + if d.Ready != nil && d.Ready.Merged != nil && d.Ready.MergeCommit != "" { + return d.Ready.MergeCommit, *d.Ready.Merged + } + return "", time.Time{} +} + // newerMergeOf says whether a merge into the same trunk of d's repository was heard that landed after d's. func (h *Holder) newerMergeOf(d *Delivery) bool { for _, o := range h.deliveries { - if o != d && o.Repository == d.Repository && o.Base == d.Base && o.MergedAs != "" && !sameCommit(o.MergedAs, d.MergedAs) && - o.MergedAt.After(d.MergedAt) { + if commit, at := mergeOf(o); o != d && o.Repository == d.Repository && o.Base == d.Base && commit != "" && + !sameCommit(commit, d.MergedAs) && at.After(d.MergedAt) { return true } } return false } + +// olderHead says whether the head source.main holds for a repository is a merge the gate knows landed before at, so a merge +// at that time may take its place. A head it does not know (written before its records, or pruned with its delivery) is +// older only when at is within the time final records are kept: a pruned merge ended before then (issue 888, review). +// Takes the lock. +func (h *Holder) olderHead(repository, base, head string, at time.Time) bool { + h.mu.Lock() + defer h.mu.Unlock() + for _, o := range h.deliveries { + if commit, when := mergeOf(o); o.Repository == repository && o.Base == base && commit != "" && sameCommit(commit, head) { + return !when.After(at) + } + } + return at.After(h.Now().Add(-keptFinal)) +}