From 5bc95786348691446c16c68b05c333fff154410e Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 23 Oct 2026 12:23:40 +0200 Subject: [PATCH 1/2] Move source.main for a merge heard only from the forge, so the checker judges on the trunk that is (issue 888) The checker reads the trunk only from the gate's source.main, and the gate moved it only in finish, for a merge it handed to the forge bridge on an Approve. A pull request merged at the forge directly moved every other delivery's trunk (trunkMoved) and left source.main behind, so the checker kept judging candidates on the trunk before it (catalogue 89e15c7b after 6c0024e4, 2026-10-23). settleForgeMerges now writes such a merge into source.main by compare-and-set, never a merge replayed as history or older than one of its repository already heard; an approved delivery merged at the forge before the gate handed it is moved by finish with its group, once. --- .../cmd/mesh-delivery-gate/issue888_test.go | 103 ++++++++++++++++++ .../cmd/mesh-delivery-gate/merges.go | 69 +++++++++++- .../cmd/mesh-delivery-gate/model.go | 5 +- .../cmd/mesh-delivery-gate/ready.go | 1 + 4 files changed, 176 insertions(+), 2 deletions(-) create mode 100644 modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue888_test.go 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 new file mode 100644 index 00000000..54d777e0 --- /dev/null +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue888_test.go @@ -0,0 +1,103 @@ +package main + +import ( + "testing" + "time" + + sdk "git.novox.be/novox/mesh-sdk/go/delivery" +) + +// Issue 888: the checker judges on the trunk source.main names (mesh-delivery modules/mesh-delivery-checker main.go), and the +// gate moved it only for a merge it handed to the forge bridge (finish). A merge made at the forge directly moved every other +// delivery's trunk (trunkMoved) and left source.main, so the checker kept judging on the trunk before it. + +const ( + i888Other = "novox/other" + i888OtherH = "0000000000000000000000000000000000000001" + i888App = "novox/app" + i888Head1 = "1111111111111111111111111111111111111111" + i888Head2 = "2222222222222222222222222222222222222222" + i888Merge1 = "3333333333333333333333333333333333333333" + i888Merge2 = "4444444444444444444444444444444444444444" +) + +func i888MergedAt(repo string, number int, head, merge string, at time.Time) PullEvent { + p := merged(repo, number, head, merge) + p.MergedAt = at.UTC().Format(time.RFC3339) + return p +} + +// 888/AC1: a merge into main the gate did not hand is heard from the forge: source.main holds it, and keeps the others. +func TestAMergeHeardOnlyFromTheForgeMovesSourceMain(t *testing.T) { + w := newWorld(t) + w.source.main = &sdk.Source{V: sdk.Version, Group: "earlier", Heads: map[string]string{i888Other: i888OtherH}} + w.h.PullUpdated(pr(i888App, 3, i888Head1, "fix/x", "internal/a.go")) + w.settleAll() + w.h.PullMerged(i888MergedAt(i888App, 3, i888Head1, i888Merge1, w.now.Add(-time.Minute))) + w.settleAll() + if w.source.main == nil || w.source.main.Heads[i888App] != i888Merge1 || w.source.main.Heads[i888Other] != i888OtherH { + t.Fatalf("source.main is %+v; wanted %s at %s and %s kept", w.source.main, i888App, i888Merge1, i888Other) + } + n := len(w.source.moves) + w.settleAll() + w.h.PullMerged(i888MergedAt(i888App, 3, i888Head1, i888Merge1, w.now.Add(-time.Minute))) // heard again + w.settleAll() + if len(w.source.moves) != n || n != 1 { + t.Fatalf("source.main moved %d then %d times; wanted once", n, len(w.source.moves)) + } +} + +// 888/AC2: a group the gate merged through the bridge moves source.main once, in finish, though the forge's word of each +// member's merge is heard too. +func TestAGroupTheGateMergedMovesSourceMainOnceThoughTheForgeSaysItsMerges(t *testing.T) { + w := approvedGroup(t) + allGreen(w) + w.op.answer(w.op.open()[0], OptionApprove) + w.settleAll() + w.settleAll() + if len(w.forge.merges) != 2 || len(w.source.moves) != 1 { + t.Fatalf("merges %v, source.main moved %d times", w.forge.merges, len(w.source.moves)) + } + commit := w.source.moves[0].Heads[gCat] + w.h.PullMerged(i888MergedAt(gCtl, 1, gHc, w.source.moves[0].Heads[gCtl], w.now)) + w.h.PullMerged(i888MergedAt(gCat, 2, gHk, commit, w.now)) + w.settleAll() + if len(w.source.moves) != 1 { + t.Fatalf("source.main moved %d times; wanted once, by the group: %+v", len(w.source.moves), w.source.moves) + } +} + +// 888/AC3: an older merge heard after a newer one, or replayed as history, never moves source.main backwards. +func TestAnOlderMergeHeardLaterNeverMovesSourceMainBack(t *testing.T) { + w := newWorld(t) + w.h.PullMerged(i888MergedAt(i888App, 4, i888Head2, i888Merge2, w.now.Add(-time.Minute))) + w.settleAll() + w.h.PullMerged(i888MergedAt(i888App, 3, i888Head1, i888Merge1, w.now.Add(-2*time.Minute))) + w.settleAll() + if w.source.main == nil || w.source.main.Heads[i888App] != i888Merge2 || len(w.source.moves) != 1 { + t.Fatalf("source.main is %+v after %d moves; wanted the newer merge %s, moved once", w.source.main, len(w.source.moves), i888Merge2) + } + + h := newWorld(t) + h.source.main = &sdk.Source{V: sdk.Version, Group: "now", Heads: map[string]string{i888App: i888Merge2}} + h.h.PullMerged(i888MergedAt(i888App, 3, i888Head1, i888Merge1, h.now.Add(-4*24*time.Hour))) + h.settleAll() + if h.source.main.Heads[i888App] != i888Merge2 || len(h.source.moves) != 0 { + t.Fatalf("a merge replayed as history moved source.main to %+v", h.source.main) + } +} + +// 888/AC4: an approved group whose members were merged at the forge before the gate handed them moves source.main once. +func TestAnApprovedGroupMergedAtTheForgeMovesSourceMainOnce(t *testing.T) { + w := approvedGroup(t) + allGreen(w) + w.op.answer(w.op.open()[0], OptionApprove) + mc, mk := "5555555555555555555555555555555555555555", "6666666666666666666666666666666666666666" + w.h.PullMerged(i888MergedAt(gCtl, 1, gHc, mc, w.now)) + w.h.PullMerged(i888MergedAt(gCat, 2, gHk, mk, w.now)) + w.settleAll() + w.settleAll() + if len(w.source.moves) != 1 || w.source.main.Heads[gCtl] != mc || w.source.main.Heads[gCat] != mk { + t.Fatalf("source.main moved %d times to %+v; wanted once, both members at their merge commits", len(w.source.moves), w.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 4075efff..c8a5abaf 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/merges.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/merges.go @@ -346,7 +346,8 @@ func (h *Holder) settleMerges() []func() { } } }) - case r.Merged != nil && !r.SourceMoved && !done[key]: + case (r.Merged != nil || d.MergedAs != "") && !r.SourceMoved && !done[key]: + // Merged through the bridge, or at the forge directly after the Approve (issue 888): the group is moved once. if !h.allMerged(r.Heads) { continue } @@ -420,3 +421,69 @@ func (h *Holder) finish(name, key string, approved map[string]string, last strin } h.Logf("[mesh-delivery] group %s is merged whole: source.main holds %s", name, strings.Join(members, ", ")) } + +// gateMerges says whether finish moves source.main for a delivery: one the operator approved through an ask, whose merge the +// gate hands to the forge bridge. +func gateMerges(d *Delivery) bool { + return d.Ready != nil && d.Ready.Approved != nil && d.Ready.AskID != "" && len(d.Ready.Heads) > 0 +} + +// settleForgeMerges moves source.main for each merge into the trunk heard only from the forge (issue 888): a pull request +// merged at the forge directly, not handed by the gate on an Approve. Every other delivery's trunk moved with it (trunkMoved), +// and the checker judges on the trunk source.main names, so source.main moves too. A merge replayed as history, or older than +// a merge of its repository already heard, is never written: source.main never moves backwards. Under the lock; the writing +// happens outside it. +func (h *Holder) settleForgeMerges() []func() { + var out []func() + for _, d := range h.deliveries { + if d.MergedAs == "" || d.InSource || gateMerges(d) { + continue + } + if d.Base != MergeBranch || d.History != "" || h.newerMergeOf(d) { + d.InSource = true // not the trunk, or not its head: nothing to write + h.keep(d) + continue + } + id, repository, commit, group := d.ID, d.Repository, d.MergedAs, 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) { + return next, false // there already: the bridge's word and the forge's are one merge + } + for r, head := range prev.Heads { + if r != repository { + next.Heads[r] = head + } + } + } + return next, true + }) + h.mu.Lock() + defer h.mu.Unlock() + if err != nil { + h.Logf("[mesh-delivery] %s merged at the forge, and its place in the source cannot be written yet: %v; asked again in %s", id, err, askAgainAfter) + h.again("source.main, moved for a merge heard from the forge") + return + } + if m := h.deliveries[id]; m != nil { + m.InSource = true + h.keep(m) + } + h.Logf("[mesh-delivery] %s merged at the forge: source.main holds %s at %s", id, repository, shortOf(commit, 12)) + }) + } + return out +} + +// 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) { + return true + } + } + return false +} diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/model.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/model.go index 5eb6cef3..32ccda2a 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/model.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/model.go @@ -84,7 +84,10 @@ type Delivery struct { Others []PullEvent `json:"others,omitempty"` MergedAs string `json:"merged_as,omitempty"` MergedAt time.Time `json:"merged_at"` - HeldWhy string `json:"held_why,omitempty"` + // InSource says the merge was looked at for source.main: written there, or left as older than the trunk's head (issue 888). + // A merge the gate handed to the bridge on an Approve is moved there by finish instead (merges.go). + InSource bool `json:"in_source,omitempty"` + HeldWhy string `json:"held_why,omitempty"` // BackupsAgainAfter is the revision of its walk when a person released it for a failed backup: a record at that // revision or before still holds the failure the controller was asked to clear, and does not hold it again // (novox/hq ADR 0404, issue 1068). diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready.go index ca9c08b2..bda03ce7 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready.go @@ -267,6 +267,7 @@ func (h *Holder) readyAsks(now time.Time) []func() { } } asks = append(asks, h.settleMerges()...) + asks = append(asks, h.settleForgeMerges()...) asks = append(asks, h.withdrawnKeys()...) asks = append(h.cancels, asks...) h.cancels = nil From dfcdf234c564ce0cba991a9e48a9a4c8e592db1b Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 23 Oct 2026 12:36:58 +0200 Subject: [PATCH 2/2] 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)) +}