Merge remote-tracking branch 'origin/main' into fix/888-forge-merge-moves-source

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).

modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue888_test.go

@@ -0,0 +1,134 @@
+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)
+	}
+}
+
+// 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)
+	}
+}

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,100 @@ 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, 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 {
+					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
+						}
+					}
+				}
+				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
+}
+
+// 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 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))
+}

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).

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