Match a fix to a delivered revision by what the gate says of its merge; keep #474's source catch-up at start (issues 1946, 1726)
This commit is contained in:
@@ -69,6 +69,9 @@ type ChecksOf struct {
|
||||
Since string `json:"since"`
|
||||
Waits string `json:"waits,omitempty"`
|
||||
Group string `json:"group,omitempty"`
|
||||
// MergedAs is the commit its merge landed as on the trunk, once merged: what the delivery view's `delivered` event is
|
||||
// matched to a fix by (ADR 0539 clause 17).
|
||||
MergedAs string `json:"merged_as,omitempty"`
|
||||
// Opened and MergedAt are what the issue tracker measures a fix's time by (novox/hq ADR 0313): when its pull request
|
||||
// was first heard, and when it merged.
|
||||
Opened string `json:"opened,omitempty"`
|
||||
@@ -112,7 +115,7 @@ func (h *Holder) Checks(repository string, number int, commit string) (*ChecksAn
|
||||
}
|
||||
if d != nil {
|
||||
a.Delivery = &ChecksOf{ID: d.ID, State: d.State, Since: d.Since.UTC().Format(time.RFC3339), Waits: d.waits,
|
||||
Group: d.Group}
|
||||
Group: d.Group, MergedAs: d.MergedAs}
|
||||
if !d.opened.IsZero() {
|
||||
a.Delivery.Opened = d.opened.UTC().Format(time.RFC3339)
|
||||
}
|
||||
|
||||
@@ -142,7 +142,7 @@ func ViewBody(d *Delivery, g *Group) string {
|
||||
for _, t := range d.Transitions[from:] {
|
||||
fmt.Fprintf(&b, "- %s %s → %s (%s): %s\n", t.At.UTC().Format("2006-01-02 15:04"), orNothing(t.From), t.To, t.Event, t.Why)
|
||||
}
|
||||
b.WriteString("\nThe commit's note under `refs/notes/mesh-plan` keeps every transition: `git log --notes=mesh-plan`.\n")
|
||||
b.WriteString("\nThe commit's note under `refs/notes/mesh-plan` keeps every transition of this change up to its merge: `git log --notes=mesh-plan`.\n")
|
||||
return b.String()
|
||||
}
|
||||
|
||||
|
||||
@@ -43,6 +43,9 @@ type Holder struct {
|
||||
// withdrawn are the ask keys withdrawn since the last look (approval.go withdrawAsk).
|
||||
withdrawn map[string]bool
|
||||
|
||||
// catchUp asks the next look to weigh again the newest merge of each repository against source.main (SourceCatchUp).
|
||||
catchUp bool
|
||||
|
||||
mu sync.Mutex
|
||||
deliveries map[string]*Delivery
|
||||
groups map[string]*Group
|
||||
|
||||
@@ -0,0 +1,75 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
sdk "git.novox.be/novox/mesh-sdk/go/delivery"
|
||||
)
|
||||
|
||||
// Issue 1726 (2026-10-24, mesh-catalog #471 and #472), kept in release 6 where no seat holder arrives to run a catch-up: the
|
||||
// checker takes a repository's trunk head only from source.main, and the gate moves it only on a merge it hears. A merge marked
|
||||
// in the source and never written (as a gate before the fix left #471) reaches source.main when the gate starts, once; one older
|
||||
// than the head source.main holds never moves it back.
|
||||
|
||||
const (
|
||||
i1726Repo = "novox/mesh-catalog"
|
||||
i1726Head1 = "aaaa000000000000000000000000000000000001"
|
||||
i1726Merge1 = "dddd000000000000000000000000000000000002"
|
||||
i1726Merge2 = "bbbb000000000000000000000000000000000003"
|
||||
)
|
||||
|
||||
func TestStartingWritesTheNewestMergeOfEachRepositoryNotYetInSourceMain(t *testing.T) {
|
||||
w := newWorld(t)
|
||||
w.h.PullUpdated(pr(i1726Repo, 470, i1726Head1, "fix/a", "modules/app/a.go"))
|
||||
w.h.PullMerged(i888MergedAt(i1726Repo, 470, i1726Head1, i1726Merge1, w.now.Add(-time.Hour)))
|
||||
const head2 = "cccc000000000000000000000000000000000001"
|
||||
w.h.PullUpdated(pr(i1726Repo, 471, head2, "fix/b", "modules/app/b.go"))
|
||||
w.h.PullMerged(i888MergedAt(i1726Repo, 471, head2, i1726Merge2, w.now.Add(-time.Minute)))
|
||||
w.settleAll()
|
||||
if got := w.source.main.Heads[i1726Repo]; got != i1726Merge2 {
|
||||
t.Fatalf("the heard merges left source.main at %s", got)
|
||||
}
|
||||
// What a gate before the fix left: the merge marked in the source, source.main still at the merge before.
|
||||
w.source.main = &sdk.Source{V: sdk.Version, Group: "before", Heads: map[string]string{i1726Repo: i1726Merge1, "novox/other": "b0"}}
|
||||
w.settleAll()
|
||||
if got := w.source.main.Heads[i1726Repo]; got != i1726Merge1 {
|
||||
t.Fatalf("source.main moved to %s with no catch-up asked", got)
|
||||
}
|
||||
w.h.SourceCatchUp()
|
||||
w.settleAll()
|
||||
if got := w.source.main.Heads[i1726Repo]; got != i1726Merge2 || w.source.main.Heads["novox/other"] != "b0" {
|
||||
t.Fatalf("source.main is %+v after the catch-up; wanted %s at %s and the other kept", w.source.main.Heads, i1726Repo, i1726Merge2)
|
||||
}
|
||||
n := len(w.source.moves)
|
||||
w.h.SourceCatchUp()
|
||||
w.settleAll()
|
||||
if len(w.source.moves) != n {
|
||||
t.Fatalf("a second catch-up moved source.main again: %d moves, then %d", n, len(w.source.moves))
|
||||
}
|
||||
|
||||
o := newWorld(t)
|
||||
o.source.main = &sdk.Source{V: sdk.Version, Group: "newer", Heads: map[string]string{i1726Repo: i1726Merge2}}
|
||||
o.h.PullUpdated(pr(i1726Repo, 471, head2, "fix/b", "modules/app/b.go"))
|
||||
o.h.PullMerged(i888MergedAt(i1726Repo, 471, head2, i1726Merge2, o.now.Add(-time.Minute)))
|
||||
o.h.PullMerged(i888MergedAt(i1726Repo, 470, i1726Head1, i1726Merge1, o.now.Add(-time.Hour)))
|
||||
o.settleAll()
|
||||
o.h.SourceCatchUp()
|
||||
o.settleAll()
|
||||
if got := o.source.main.Heads[i1726Repo]; got != i1726Merge2 {
|
||||
t.Fatalf("the catch-up moved source.main back to %s", got)
|
||||
}
|
||||
}
|
||||
|
||||
// A delivery kept with no merge time (adopted from a walk by an earlier owner) is dated by when the gate made it, so it does not
|
||||
// make every older merge look newer, and the catch-up at the start weighs it.
|
||||
func TestAMergeKeptWithNoMergeTimeIsDatedByWhenTheGateMadeIt(t *testing.T) {
|
||||
d := &Delivery{MergedAs: i1726Merge2, Created: time.Date(2026, 10, 24, 12, 0, 0, 0, time.UTC)}
|
||||
if got := mergedWhen(d); !got.Equal(d.Created) {
|
||||
t.Fatalf("dated %s", got)
|
||||
}
|
||||
d.MergedAt = d.Created.Add(-time.Hour)
|
||||
if got := mergedWhen(d); !got.Equal(d.MergedAt) {
|
||||
t.Fatalf("dated %s with a merge time", got)
|
||||
}
|
||||
}
|
||||
@@ -63,7 +63,7 @@ func TestATransitionSaysWhenItsPullRequestOpenedAndMerged(t *testing.T) {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if a.Delivery == nil || a.Delivery.Opened != opened.UTC().Format(time.RFC3339) ||
|
||||
a.Delivery.MergedAt != mergedWhen.UTC().Format(time.RFC3339) {
|
||||
a.Delivery.MergedAt != mergedWhen.UTC().Format(time.RFC3339) || a.Delivery.MergedAs != "merge9aaaaaa" {
|
||||
t.Errorf("checks says %+v", a.Delivery)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -154,7 +154,7 @@ func loadKept(h *Holder, l *listening) {
|
||||
func readOnStart(h *Holder) {
|
||||
h.wantForge()
|
||||
h.hearAsks()
|
||||
h.Poke()
|
||||
h.SourceCatchUp() // a merge missed or marked unwritten while the gate was away reaches source.main, and so the checker (issue 1726)
|
||||
}
|
||||
|
||||
// handle takes one event, then asks for a look at every step the event makes possible (Work, React). An event that cannot be read is
|
||||
@@ -299,9 +299,9 @@ func tools(h *Holder, l *listening) []stdio.Tool {
|
||||
Run: func(a map[string]any) (any, error) { return h.Show(strArg(a, "id")) }},
|
||||
{Name: seat + "groups",
|
||||
Description: "Every delivery group: its members, its composed check and its state, derived from its members. " +
|
||||
"A group has no order: it is merged in one pass and delivered by one plan, whose tiers come from the module " +
|
||||
"graph (novox/hq ADR 0418). On mesh/check its check is the checker's one verdict at every member's head (ADR 0447); " +
|
||||
"composed_gate says its members' gate is the controller's composed check's (ADR 0403), for a base still on it.",
|
||||
"A group has no order: it is merged in one pass, and moves source.main once (novox/hq ADR 0418, ADR 0539 clause 17); what " +
|
||||
"reached the machines is the delivery view's. On mesh/check its check is the checker's one verdict at every member's head " +
|
||||
"(ADR 0447); composed_gate says its members' gate is the controller's composed check's (ADR 0403), for a base still on it.",
|
||||
Input: map[string]any{"type": "object", "properties": map[string]any{
|
||||
"all": map[string]any{"type": "string", "enum": []string{"true", "false"}}}},
|
||||
Run: func(a map[string]any) (any, error) { return h.Groups(boolArg(a, "all")), nil }},
|
||||
|
||||
@@ -435,8 +435,13 @@ func gateMerges(d *Delivery) bool {
|
||||
// happens outside it.
|
||||
func (h *Holder) settleForgeMerges() []func() {
|
||||
var out []func()
|
||||
again := map[*Delivery]bool{}
|
||||
if h.catchUp {
|
||||
h.catchUp = false
|
||||
again = h.newestInSource()
|
||||
}
|
||||
for _, d := range h.deliveries {
|
||||
if d.MergedAs == "" || d.InSource || gateMerges(d) {
|
||||
if d.MergedAs == "" || (d.InSource && !again[d]) || gateMerges(d) {
|
||||
continue
|
||||
}
|
||||
if d.Base != MergeBranch || d.replayed() || h.newerMergeOf(d) {
|
||||
@@ -444,7 +449,7 @@ func (h *Holder) settleForgeMerges() []func() {
|
||||
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)
|
||||
id, repository, base, commit, at, group := d.ID, d.Repository, d.Base, d.MergedAs, mergedWhen(d), 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}}
|
||||
@@ -495,7 +500,7 @@ func (d *Delivery) replayed() bool {
|
||||
// (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
|
||||
return d.MergedAs, mergedWhen(d)
|
||||
}
|
||||
if d.Ready != nil && d.Ready.Merged != nil && d.Ready.MergeCommit != "" {
|
||||
return d.Ready.MergeCommit, *d.Ready.Merged
|
||||
@@ -507,7 +512,7 @@ func mergeOf(d *Delivery) (string, time.Time) {
|
||||
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) {
|
||||
!sameCommit(commit, d.MergedAs) && at.After(mergedWhen(d)) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
@@ -528,3 +533,43 @@ 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: its merge time (the forge's), else when the gate made the delivery. A
|
||||
// zero time made every other merge of the repository look newer, so such a merge never moved source.main (issue 1726).
|
||||
func mergedWhen(d *Delivery) time.Time {
|
||||
if !d.MergedAt.IsZero() {
|
||||
return d.MergedAt
|
||||
}
|
||||
return d.Created
|
||||
}
|
||||
|
||||
// SourceCatchUp asks the next look to weigh again, once, the newest merge into the trunk of each repository though it is
|
||||
// marked in the source (issue 1726, mesh-catalog #474, kept here because no seat holder arrives in release 6 to run it): what
|
||||
// the gate missed while it was not running, or marked without writing, reaches source.main. The write never moves a repository
|
||||
// back (olderHead), and one already there is not written again.
|
||||
func (h *Holder) SourceCatchUp() {
|
||||
h.mu.Lock()
|
||||
h.catchUp = true
|
||||
h.mu.Unlock()
|
||||
h.Poke()
|
||||
}
|
||||
|
||||
// newestInSource is, for each repository, its newest merge into the trunk heard from the forge, marked in the source. Under the lock.
|
||||
func (h *Holder) newestInSource() map[*Delivery]bool {
|
||||
newest := map[string]*Delivery{}
|
||||
for _, d := range h.deliveries {
|
||||
if d.MergedAs == "" || !d.InSource || gateMerges(d) || d.Base != MergeBranch || d.replayed() {
|
||||
continue
|
||||
}
|
||||
if o := newest[d.Repository]; o == nil || mergedWhen(d).After(mergedWhen(o)) {
|
||||
newest[d.Repository] = d
|
||||
}
|
||||
}
|
||||
out := map[*Delivery]bool{}
|
||||
for _, d := range newest {
|
||||
if !h.newerMergeOf(d) {
|
||||
out[d] = true
|
||||
}
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
@@ -131,3 +131,45 @@ func TestANewerRevisionCoversTheOlderMergesOfItsRepositoryAsTheForgeSaysAncestry
|
||||
t.Fatalf("another repository's head covered it: %q", v.Delivery)
|
||||
}
|
||||
}
|
||||
|
||||
// A fix kept with only its merged flag (heard through `checks`, or kept before release 6) is matched to a delivered revision
|
||||
// by asking the delivery for its merge: the group and the commit it landed as.
|
||||
func TestAFixKeptWithoutItsMergeIsMatchedByAskingTheDeliveryForIt(t *testing.T) {
|
||||
h := newHarness(t)
|
||||
n := fixIssue(t, h, "mesh-catalog#40")
|
||||
// Kept as a fix the tracker heard merged and nothing more: no group, no commit, no repository.
|
||||
if _, err := h.change(n, Act{Op: "delivery", By: Seat}, func(i *Issue) error {
|
||||
at := time.Date(2026, 10, 24, 12, 0, 0, 0, time.UTC)
|
||||
i.Fixes = append(i.Fixes, Fix{PullRequest: "mesh-catalog#40", State: "published", Merged: true, MergedAt: &at, Since: at})
|
||||
return nil
|
||||
}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
asked := 0
|
||||
h.Deliveries = func(repository, number string) (Delivery, error) {
|
||||
asked++
|
||||
return Delivery{State: "merged", Merged: true, Repository: "novox/mesh-catalog", Group: "feat/r6", MergedAs: "9999999999990000"}, nil
|
||||
}
|
||||
viewSays(t, h, 20, "feat/r6", nil)
|
||||
if asked == 0 {
|
||||
t.Fatal("the delivery was not asked for the merge of a fix kept without it")
|
||||
}
|
||||
if v, _ := h.View(n); v.Delivery != "delivered to every machine it should reach" {
|
||||
t.Fatalf("%q", v.Delivery)
|
||||
}
|
||||
// What `checks` says is carried: group, commit and repository reach the fix when it is heard through the delivery.
|
||||
h2 := newHarness(t)
|
||||
m := h2.open(t, "x")
|
||||
h2.Deliveries = func(string, string) (Delivery, error) {
|
||||
return Delivery{State: "merged", Merged: true, Since: time.Date(2026, 10, 24, 12, 0, 0, 0, time.UTC), Repository: "novox/mesh-catalog",
|
||||
Group: "feat/q", MergedAs: "8888888888880000"}, nil
|
||||
}
|
||||
fixedAtFixing(t, h2, m, "mesh-catalog#41")
|
||||
if err := h2.OnMoved(stdio.Envelope{Body: []byte(`{"number":` + itoa(int(m)) + `}`)}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
f := fixOf(get(t, h2, m), "mesh-catalog#41")
|
||||
if f == nil || f.Group != "feat/q" || f.MergedAs != "8888888888880000" || f.Repository != "novox/mesh-catalog" {
|
||||
t.Fatalf("heard through checks the fix keeps %+v", f)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -453,6 +453,9 @@ func (t *Tracker) OnDelivered(e stdio.Envelope) error {
|
||||
for n, is := range all {
|
||||
for _, pr := range liveFixes(is) {
|
||||
f := fixOf(is, pr)
|
||||
if f != nil && f.Merged && !contains(is.Delivered, pr) && f.MergedAs == "" {
|
||||
t.fillMerge(n, f, pr)
|
||||
}
|
||||
if f == nil || !f.Merged || contains(is.Delivered, pr) || !(deliveredBy(f, body.Group, body.Heads) || t.coveredBy(f, body.Heads)) {
|
||||
continue
|
||||
}
|
||||
@@ -482,6 +485,40 @@ func (t *Tracker) OnDelivered(e stdio.Envelope) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// fillMerge learns what the gate says of the merge of a fix kept without it (heard through `checks`, or kept before release 6
|
||||
// with only its merged flag): the repository, the group and the commit its merge landed as, asked of the delivery. A delivery
|
||||
// that cannot be asked, or does not say, leaves the fix as it was; it is asked again at the next event. f is updated in place.
|
||||
func (t *Tracker) fillMerge(n uint64, f *Fix, pr string) {
|
||||
if t.Deliveries == nil {
|
||||
return
|
||||
}
|
||||
repo, number, _ := strings.Cut(pr, "#")
|
||||
d, err := t.Deliveries(repo, number)
|
||||
if err != nil {
|
||||
t.Logf("[mesh-issues] the merge of %s (issue %d) could not be asked of the delivery: %v", pr, n, err)
|
||||
return
|
||||
}
|
||||
if d.MergedAs == "" {
|
||||
return
|
||||
}
|
||||
if _, err := t.change(n, Act{Op: "delivery", By: Seat}, func(i *Issue) error {
|
||||
if g := fixOf(i, pr); g != nil {
|
||||
g.MergedAs, g.Group = d.MergedAs, d.Group
|
||||
if d.Repository != "" {
|
||||
g.Repository = d.Repository
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}); err != nil {
|
||||
t.Logf("[mesh-issues] the merge of %s (issue %d) could not be kept: %v", pr, n, err)
|
||||
return
|
||||
}
|
||||
f.MergedAs, f.Group = d.MergedAs, d.Group
|
||||
if d.Repository != "" {
|
||||
f.Repository = d.Repository
|
||||
}
|
||||
}
|
||||
|
||||
// busContains asks the forge's gitea_contains over the bus: whether commit is an ancestor of in, or it.
|
||||
func busContains(owner, repo, commit, in string) (bool, error) {
|
||||
raw, err := stdio.Ask("gitea.gitea_contains", map[string]any{"owner": owner, "repo": repo, "commit": commit, "in": in})
|
||||
@@ -568,11 +605,16 @@ type Delivery struct {
|
||||
Class string
|
||||
Opened *time.Time
|
||||
MergedAt *time.Time
|
||||
// Repository (owner/name), Group and MergedAs are what the gate says of the merge, which the view's `delivered` is matched by.
|
||||
Repository string
|
||||
Group string
|
||||
MergedAs string
|
||||
}
|
||||
|
||||
// heard is what a delivery answered, as a fix keeps it.
|
||||
func (d Delivery) heard(since time.Time) heardFix {
|
||||
return heardFix{State: d.State, Why: d.Why, Merged: d.Merged, At: since, Class: d.Class, Opened: d.Opened, MergedAt: d.MergedAt}
|
||||
return heardFix{State: d.State, Why: d.Why, Merged: d.Merged, At: since, Class: d.Class, Opened: d.Opened, MergedAt: d.MergedAt,
|
||||
Repository: d.Repository, Group: d.Group, MergedAs: d.MergedAs}
|
||||
}
|
||||
|
||||
// nonZero is a moment said, or nil for one not said.
|
||||
@@ -602,6 +644,8 @@ func busDeliveryState(owner string) DeliveryState {
|
||||
Class string `json:"class"`
|
||||
Opened *time.Time `json:"opened"`
|
||||
MergedAt *time.Time `json:"merged_at"`
|
||||
Group string `json:"group"`
|
||||
MergedAs string `json:"merged_as"`
|
||||
} `json:"delivery"`
|
||||
Pull struct {
|
||||
Merged bool `json:"merged"`
|
||||
@@ -611,7 +655,8 @@ func busDeliveryState(owner string) DeliveryState {
|
||||
return Delivery{}, fmt.Errorf("mesh-delivery's checks answered no delivery: %.200s", raw)
|
||||
}
|
||||
return Delivery{State: out.Delivery.State, Why: out.Delivery.Waits, Since: out.Delivery.Since.UTC(), Merged: out.Pull.Merged,
|
||||
Class: out.Delivery.Class, Opened: nonZero(out.Delivery.Opened), MergedAt: nonZero(out.Delivery.MergedAt)}, nil
|
||||
Class: out.Delivery.Class, Opened: nonZero(out.Delivery.Opened), MergedAt: nonZero(out.Delivery.MergedAt),
|
||||
Repository: strings.ToLower(owner + "/" + repository), Group: out.Delivery.Group, MergedAs: out.Delivery.MergedAs}, nil
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user