From 68b5febee7c6576cc279e1ce4413d9f3cca5331b Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 1 Nov 2026 18:05:26 +0100 Subject: [PATCH 1/2] Keep the freshest mirrors first and a tail of four more in pacman's shared list, so a node has a fallback (ADR 0586, issue 1876) Claude-Session: https://claude.ai/code/session_01NcbgfJ9Gum47zkFKWRaovL --- .../pacman/cmd/pacman-tools/busshapes_test.go | 2 +- modules/pacman/cmd/pacman-tools/mirrors.go | 56 ++++++++++++++---- .../pacman/cmd/pacman-tools/pacman_test.go | 58 +++++++++++++++++-- modules/pacman/cmd/pacman-tools/shapes.go | 5 +- 4 files changed, 102 insertions(+), 19 deletions(-) diff --git a/modules/pacman/cmd/pacman-tools/busshapes_test.go b/modules/pacman/cmd/pacman-tools/busshapes_test.go index ac26c9e6..b4e84fba 100644 --- a/modules/pacman/cmd/pacman-tools/busshapes_test.go +++ b/modules/pacman/cmd/pacman-tools/busshapes_test.go @@ -35,7 +35,7 @@ var busFields = map[string][]string{ "Record": {"node", "run", "at", "installed", "kernel", "failed_units", "dkms"}, "DKMSModule": {"module", "version", "kernels"}, "Round": {"round", "opened", "opened_by", "order", "mirrors", "mirrors_note"}, - "MirrorList": {"servers", "ranked_at", "ranked_by", "options", "newest_sync", "round"}, + "MirrorList": {"servers", "ranked_at", "ranked_by", "options", "newest_sync", "freshest", "round"}, "RoundNode": {"round", "node", "run", "pass"}, "Pass": {"reason", "ask", "asked", "answer", "answered"}, "Ask": {"id", "node", "act", "at", "of", "remove_added", "round", "pass", "reason"}, diff --git a/modules/pacman/cmd/pacman-tools/mirrors.go b/modules/pacman/cmd/pacman-tools/mirrors.go index 27564916..715a798c 100644 --- a/modules/pacman/cmd/pacman-tools/mirrors.go +++ b/modules/pacman/cmd/pacman-tools/mirrors.go @@ -24,6 +24,7 @@ import ( "fmt" "os" "regexp" + "sort" "strings" "time" @@ -165,20 +166,48 @@ func ParseReflectorInfo(text string) []rankedMirror { return out } -// Freshest keeps, in their order, only the mirrors whose last sync is no older than the newest found (ADR 0585 §2), so -// a mirror lagging behind the others is never in the list; one whose last sync is unknown is left out. -func Freshest(ranked []rankedMirror) (servers []string, newest time.Time) { +// TailLength is how many of the next freshest mirrors follow the freshest ones in the list (ADR 0586 §1). +const TailLength = 4 + +// Freshest orders the mirrors by last sync, newest first, in reflector's order among equals, and keeps every mirror +// whose last sync equals the newest found (the freshest, count of them) followed by up to TailLength of the next +// freshest (ADR 0586 §1, which changes ADR 0585 §2); one whose last sync is unknown (zero) is left out. +func Freshest(ranked []rankedMirror) (servers []string, newest time.Time, freshest int) { + var known []rankedMirror for _, m := range ranked { - if m.LastSync.After(newest) { - newest = m.LastSync + if !m.LastSync.IsZero() { + known = append(known, m) } } - for _, m := range ranked { - if !m.LastSync.IsZero() && !m.LastSync.Before(newest) { - servers = append(servers, m.URL) + sort.SliceStable(known, func(i, j int) bool { return known[i].LastSync.After(known[j].LastSync) }) + if len(known) == 0 { + return nil, newest, 0 + } + newest = known[0].LastSync + for _, m := range known { + if m.LastSync.Equal(newest) { + freshest++ + } + } + for i, m := range known { + if i >= freshest+TailLength { + break + } + servers = append(servers, m.URL) + } + return servers, newest, freshest +} + +// withoutUnknownSync zeroes the last sync of every mirror that reflector printed as the moment it ran: --info writes an +// unknown last sync (None) through gmtime(None), which is the current time, so a last sync at or after start (the +// second the run began) cannot be a real one, since the status reflector reads is older than the run. +func withoutUnknownSync(ranked []rankedMirror, start time.Time) { + start = start.UTC().Truncate(time.Second) + for i := range ranked { + if !ranked[i].LastSync.Before(start) { + ranked[i].LastSync = time.Time{} } } - return servers, newest } // reflectorRun runs reflector with args on this machine, within RankTimeout; its cache in the temporary directory, since @@ -197,11 +226,12 @@ func RankMirrors(conf []byte, node string, now time.Time, round string) (MirrorL return MirrorList{}, failure("reflector", "reflector", r) } ranked := ParseReflectorInfo(r.Stdout) - servers, newest := Freshest(ranked) + withoutUnknownSync(ranked, now) + servers, newest, freshest := Freshest(ranked) if err := checkServers(servers); err != nil { return MirrorList{}, fmt.Errorf("reflector ranked no list the mesh can use (%d mirrors read): %w", len(ranked), err) } - return MirrorList{Servers: servers, RankedAt: now.UTC(), RankedBy: node, Options: opts, Newest: newest, Round: round}, nil + return MirrorList{Servers: servers, RankedAt: now.UTC(), RankedBy: node, Options: opts, Newest: newest, Freshest: freshest, Round: round}, nil } // readMirrorList is the shared list and its revision; nil when there is none yet. @@ -379,6 +409,10 @@ func (m *Machine) MirrorsView(refresh bool) (map[string]any, error) { return nil, err } out["shared"], out["revision"] = list, rev + if list != nil { + n := min(max(list.Freshest, 1), len(list.Servers)) + out["freshest"], out["tail"] = list.Servers[:n], list.Servers[n:] + } text, ferr := m.ReadFile(Mirrorlist) if ferr != nil { out["file_error"] = ferr.Error() diff --git a/modules/pacman/cmd/pacman-tools/pacman_test.go b/modules/pacman/cmd/pacman-tools/pacman_test.go index aad2b590..f980d2b5 100644 --- a/modules/pacman/cmd/pacman-tools/pacman_test.go +++ b/modules/pacman/cmd/pacman-tools/pacman_test.go @@ -2,6 +2,7 @@ package main import ( "context" + "fmt" "strings" "testing" "time" @@ -306,7 +307,10 @@ https://fr.mirror.example/archlinux/$repo/os/$arch last_sync : None ` -// ADR 0585 §2, §4: the ranking keeps only mirrors as fresh as the newest, in reflector's order, with the module's options +// rankedAt is after every last sync in reflectorInfo: reflector ran then. +var rankedAt = time.Date(2026, 11, 1, 15, 0, 0, 0, time.UTC) + +// ADR 0586 §1, ADR 0585 §4: the ranking keeps the mirrors at the newest sync and a tail of the next freshest, in reflector's order, with the module's options // and never its --save; a ranking that fails or keeps none is no list. pacman_mirrors answers the shared list, its // ranker and age and whether this node's file matches it (the file as found when not); with refresh it asks this node's // keeper to rank again, refused while a round runs, naming it. @@ -320,18 +324,19 @@ func TestTheMeshsMirrorListIsRankedFreshAndAnsweredWithThisNodesFile(t *testing. saved := reflectorRun reflectorRun = func(_ context.Context, args ...string) Ran { asked = args; return answer } defer func() { reflectorRun = saved }() - l, err := RankMirrors([]byte(conf), "node-a", t0, "2026-W43") - if err != nil || strings.Join(l.Servers, " ") != "https://nl.mirror.example/archlinux/$repo/os/$arch https://de.mirror.example/$repo/os/$arch" || + l, err := RankMirrors([]byte(conf), "node-a", rankedAt, "2026-W43") + if err != nil || strings.Join(l.Servers, " ") != "https://nl.mirror.example/archlinux/$repo/os/$arch https://de.mirror.example/$repo/os/$arch https://be.mirror.example/archlinux/$repo/os/$arch" || l.Freshest != 2 || !l.Newest.Equal(time.Date(2026, 11, 1, 14, 32, 1, 0, time.UTC)) || l.RankedBy != "node-a" || l.Round != "2026-W43" || strings.Join(asked, " ") != "--country Belgium,Netherlands --sort rate --info" { t.Fatalf("ranked %+v (%v), reflector asked %v", l, err, asked) } for _, a := range []Ran{{Status: 1, Stderr: "error: no mirrors"}, {Stdout: "https://x.example/$repo/os/$arch\nlast_sync : None\n"}} { answer = a - if _, err := RankMirrors([]byte(conf), "node-a", t0, ""); err == nil { + if _, err := RankMirrors([]byte(conf), "node-a", rankedAt, ""); err == nil { t.Errorf("a ranking from %+v is a list", a) } } + checkFreshest(t) if checkServers([]string{"https://ok.example/$repo/os/$arch\nServer = https://evil.example/$repo/os/$arch"}) == nil { t.Error("a server carrying a second line is taken") } @@ -343,7 +348,7 @@ func TestTheMeshsMirrorListIsRankedFreshAndAnsweredWithThisNodesFile(t *testing. return Ran{Stdout: "LoadState=loaded\nUnitFileState=disabled\nActiveState=inactive\n"} }, 1000) m.Node, m.States = "node-a", readerOf(states) - m.Now = func() time.Time { return t0.Add(90 * time.Minute) } + m.Now = func() time.Time { return rankedAt.Add(90 * time.Minute) } m.ReadFile = func(string) ([]byte, error) { return file, nil } v, err := m.MirrorsView(false) if err != nil || v["file_matches"] != true || v["ranked_by"] != "node-a" || v["age"] != "1h30m0s" || v["file"] != nil || @@ -382,3 +387,46 @@ func TestConfigIsReadAsPacmanConfResolvesIt(t *testing.T) { t.Fatalf("%+v", c) } } + +// ADR 0586 §1: the freshest first, then up to four of the next freshest, newest first; an unknown last sync is left out, +// whether reflector wrote None or the moment it ran (gmtime(None)). +func checkFreshest(t *testing.T) { + t.Helper() + at := func(m int) time.Time { return time.Date(2026, 11, 1, 14, m, 0, 0, time.UTC) } + mk := func(syncs ...int) []rankedMirror { + var r []rankedMirror + for i, m := range syncs { + mm := rankedMirror{URL: fmt.Sprintf("m%d", i)} + if m >= 0 { + mm.LastSync = at(m) + } + r = append(r, mm) + } + return r + } + for _, c := range []struct { + name string + ranked []rankedMirror + servers string + fresh int + }{ + {"all fresh", mk(30, 30, 30), "m0 m1 m2", 3}, + {"one fresh and a tail of four", mk(10, 30, 20, 25, 5, 1), "m1 m3 m2 m0 m4", 1}, + {"more than four candidates", mk(30, 29, 28, 27, 26, 25, 24), "m0 m1 m2 m3 m4", 1}, + {"two fresh and the tail", mk(30, 20, 30, 10, 11, 12, 13), "m0 m2 m1 m6 m5 m4", 2}, + {"unknown last sync left out", mk(30, -1, 20, -1), "m0 m2", 1}, + {"none known", mk(-1, -1), "", 0}, + } { + got, _, fresh := Freshest(c.ranked) + if strings.Join(got, " ") != c.servers || fresh != c.fresh { + t.Errorf("%s: kept %v, %d freshest; want %q, %d", c.name, got, fresh, c.servers, c.fresh) + } + } + // reflector prints an unknown last sync as the moment it ran: at or after the start of the run is no real sync. + start := time.Date(2026, 11, 1, 15, 0, 0, 500, time.UTC) + r := []rankedMirror{{"a", at(30)}, {"b", start.Truncate(time.Second)}, {"c", start.Add(time.Second)}} + withoutUnknownSync(r, start) + if r[0].LastSync.IsZero() || !r[1].LastSync.IsZero() || !r[2].LastSync.IsZero() { + t.Errorf("unknown syncs not detected: %+v", r) + } +} diff --git a/modules/pacman/cmd/pacman-tools/shapes.go b/modules/pacman/cmd/pacman-tools/shapes.go index f79aedb3..04f51397 100644 --- a/modules/pacman/cmd/pacman-tools/shapes.go +++ b/modules/pacman/cmd/pacman-tools/shapes.go @@ -334,14 +334,15 @@ type Round struct { } // MirrorList is `mirrors.list`, the mesh's one mirror list (ADR 0585 §1): the servers in order, when and by which -// node they were ranked, the options reflector ranked them with, the newest last sync found among them (only mirrors as -// fresh as it are kept, §2), and the round it was ranked for (none for a refresh). +// node they were ranked, the options reflector ranked them with, the newest last sync found among them (the mirrors at it come first, +// then a tail of up to four fresher-than-the-rest, ADR 0586), and the round it was ranked for (none for a refresh). type MirrorList struct { Servers []string `json:"servers"` RankedAt time.Time `json:"ranked_at"` RankedBy string `json:"ranked_by"` Options []string `json:"options"` Newest time.Time `json:"newest_sync"` + Freshest int `json:"freshest"` // how many of the first servers are at the newest sync; the rest are the fallback tail (ADR 0586) Round string `json:"round,omitempty"` } -- 2.54.0 From 90f7f87c2ac180d3087659c5d229849b76f6e42f Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 1 Nov 2026 18:06:27 +0100 Subject: [PATCH 2/2] Say a failed check first in the merge ask's headline, since a merge is not gated on the check and the headline is the phone's only warning (issue 1166) Claude-Session: https://claude.ai/code/session_01NcbgfJ9Gum47zkFKWRaovL --- .../cmd/mesh-delivery-gate/approval.go | 14 ++++++++++++-- .../cmd/mesh-delivery-gate/issue1348_test.go | 12 ++++++++++++ .../cmd/mesh-delivery-gate/ready_test.go | 7 ++++++- 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval.go index c635afc6..77d9805f 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval.go @@ -169,6 +169,16 @@ func approveCaveat(c checkSaid) string { return "" } +// mergeHeadline opens the ask's headline: "Merge into main: ", and "Check failed - merge into main: " once the check failed, +// since a merge is not gated on the check and the headline is the phone's only warning (issue 1166). A passed or still +// running check keeps the plain headline. The prefix leads, so phone's cut of a long headline never drops it. +func mergeHeadline(c checkSaid) string { + if c.Key == checkSaidFail { + return "Check failed \u2014 merge into main: " + } + return "Merge into main: " +} + // composeMergeAsk is the ask for a delivery, or for a group's members together. // check is the state of the check at the heads it covers (checkAt): the ask says "passed its check" only when it did, and // otherwise that Approve merges before the check ends, or although it failed (issue 1733). @@ -193,7 +203,7 @@ func composeMergeAsk(id string, d *Delivery, group string, members []*Delivery, pullSaid(m), m.Title, shortOf(m.Commit, 12), issuesSaid([]*Delivery{m}), words, touches)) } if group == "" { - a.Headline = phone("Merge into main: "+pullSaid(d)+" "+d.Title, asks.HeadlineLength) + a.Headline = phone(mergeHeadline(check)+pullSaid(d)+" "+d.Title, asks.HeadlineLength) summary := fmt.Sprintf("%s %s%s. Approve merges it into main at %s%s; Decline leaves it unmerged.", pullSaid(d), d.Title, checkClause(check, " "), shortOf(d.Commit, 12), approveCaveat(check)) if unreviewed != "" { @@ -210,7 +220,7 @@ func composeMergeAsk(id string, d *Delivery, group string, members []*Delivery, for _, m := range members { names = append(names, pullSaid(m)) } - a.Headline = phone("Merge into main: group "+group+" ("+strings.Join(names, ", ")+")", asks.HeadlineLength) + a.Headline = phone(mergeHeadline(check)+"group "+group+" ("+strings.Join(names, ", ")+")", asks.HeadlineLength) caveat := approveCaveat(check) if caveat != "" { caveat = "," + caveat diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1348_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1348_test.go index 4555603b..98a9cac5 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1348_test.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1348_test.go @@ -3,6 +3,7 @@ package main import ( "strings" "testing" + "time" "git.novox.be/novox/mesh-sdk/go/asks" sdk "git.novox.be/novox/mesh-sdk/go/delivery" @@ -42,6 +43,17 @@ func TestAReadyGroupWritesOneAskKeyWithTodaysWordsAndCallsNoRouter(t *testing.T) !strings.Contains(ask.Ask.Details, "mesh-catalog#2") || len(ask.Ask.Options) != 2 || ask.Ask.Who != asks.Operator { t.Fatalf("the ask is not today's merge ask: %+v", ask.Ask) } + // A group's headline says a failed check first (issue 1166); a passed or a running one keeps the plain words. + members := []*Delivery{{Repository: "novox/mesh-controller", Number: 1}, {Repository: "novox/mesh-catalog", Number: 2}} + for _, c := range []struct{ key, head string }{ + {checkSaidFail, "Check failed \u2014 merge into main: group feat"}, + {checkSaidPass, "Merge into main: group feat"}, + {"running", "Merge into main: group feat"}, + } { + if h := composeMergeAsk("id", members[0], "feat/g", members, nil, checkSaid{Key: c.key, Words: "w"}, time.Time{}).Headline; !strings.HasPrefix(h, c.head) { + t.Fatalf("a %s check gives the headline %q, wanted it to start %q", c.key, h, c.head) + } + } // The act the Approve binds is the one the teller checks and the controller verifies: this ask's router id, the option, and // the heads as `owner/repo@` to the whole commit. approve, _ := ask.Ask.Option(OptionApprove) diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready_test.go index c8354af9..bfd839a2 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready_test.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready_test.go @@ -80,8 +80,9 @@ func TestAnApprovedPullRequestIsMergedThroughTheMergeTool(t *testing.T) { after time.Duration asks, open int want, not []string + head string // the ask's headline starts with it }{ - {name: "gated, passed", gated: true, state: "pass", asks: 1, + {name: "gated, passed", gated: true, state: "pass", asks: 1, head: "Merge into main: app#7 Fix it", want: []string{"app#7 Fix it passed its check. Approve merges it into main at " + readyHead[:12] + ";"}, not: []string{"before its check ends", "no verdict yet"}}, {name: "ungated, no verdict", asks: 0, want: []string{waitsCheckBeforeAsking + "its check has no verdict yet"}, not: []string{"anyway"}}, @@ -98,6 +99,7 @@ func TestAnApprovedPullRequestIsMergedThroughTheMergeTool(t *testing.T) { {name: "ungated, passed, its verdict gone", state: "pass", then: "gone", asks: 1, open: -1, want: []string{waitsCheckBeforeAsking + "its check has no verdict yet"}, not: []string{"anyway"}}, {name: "ungated, failed", state: "fail", failed: "TestThing: wanted 1, got 2", asks: 1, + head: "Check failed \u2014 merge into main: app#7 Fix it", want: []string{"its check failed: TestThing: wanted 1, got 2", "although its check failed"}, not: []string{"passed", "anyway"}}, } { t.Run(row.name, func(t *testing.T) { @@ -143,6 +145,9 @@ func TestAnApprovedPullRequestIsMergedThroughTheMergeTool(t *testing.T) { } return } + if h := v.op.ask(open[0]).Headline; row.head != "" && !strings.HasPrefix(h, row.head) { + t.Fatalf("the headline is %q; wanted it to start %q", h, row.head) + } e := v.op.ask(open[0]).Explanation for _, w := range row.want { if !strings.Contains(e, w) { -- 2.54.0