Compare commits

..
7 changed files with 39 additions and 245 deletions
@@ -411,9 +411,6 @@ func (h *Holder) approval(d *Delivery, now time.Time) (string, func()) {
}
}
check := h.checkAt(members)
if !h.checkGates() {
check = unansweredWords(check) // asked with no verdict only past its bound (verdictWaits)
}
ask := composeMergeAsk(sdk.AskingID(id, asking), d, group, members, heads, check, now)
taken := []*Delivery{}
var reads []memberRead
@@ -1,11 +1,6 @@
package main
import (
"encoding/json"
"time"
sdk "git.novox.be/novox/mesh-sdk/go/delivery"
)
import "time"
var t0 = time.Date(2026, 10, 6, 12, 0, 0, 0, time.UTC)
@@ -32,24 +27,3 @@ func statusOn(w *world, repo, commit string) string {
defer w.forge.mu.Unlock()
return w.forge.statuses[repo+"@"+commit+" mesh/delivery"]
}
// checkerSays puts the checker's verdict at these heads together into its verdicts bucket, as captured on the bus
// (mesh-delivery-checker, key `<group>.<heads hash>`).
func checkerSays(w *world, group, state, failed string, heads ...sdk.Head) {
checkerSaysWith(w, group, state, failed, nil, heads...)
}
// checkerSaysWith is checkerSays with more of the verdict's fields (its composition); the state "gone" deletes the key.
func checkerSaysWith(w *world, group, state, failed string, more map[string]any, heads ...sdk.Head) {
hash, _ := sdk.HeadsHash(heads)
if state == "gone" {
w.h.CheckerVerdictChanged(group+"."+hash, nil)
return
}
v := map[string]any{"v": 1, "group": group, "heads": hash, "state": state, "failed": map[string]string{"words": failed}}
for k, x := range more {
v[k] = x
}
value, _ := json.Marshal(v)
w.h.CheckerVerdictChanged(group+"."+hash, value)
}
@@ -77,10 +77,7 @@ type Holder struct {
unchecked map[string]unchecked
// checker are the checker's verdicts by the hash of the heads they judged, as its verdicts bucket holds them
// (onestatus.go, issue 1302); runningSince when this owner first saw each running (ADR 0548 §2, turns.go).
checker map[string]checkerVerdict
// verdictGone is when the checker's verdict at a heads hash was taken away (its key deleted): the Approve's wait for a
// verdict counts its bound from then, as from a check that starts again (verdictWaits).
verdictGone map[string]time.Time
checker map[string]checkerVerdict
runningSince map[string]time.Time
// unmergeableSince is when the forge first said a group's member does not merge now, by group and member (issue 1275,
// group_merge.go): within forgeMergeGrace that is the forge still working out mergeability, asked again shortly.
@@ -144,7 +141,7 @@ func (h *Holder) init() {
h.lostRead = map[string]time.Time{}
h.unchecked = map[string]unchecked{}
h.checker = map[string]checkerVerdict{}
h.runningSince, h.turnsHeld, h.verdictGone = map[string]time.Time{}, map[string]sdk.Turn{}, map[string]time.Time{}
h.runningSince, h.turnsHeld = map[string]time.Time{}, map[string]sdk.Turn{}
h.reach = map[string]sdk.Reach{}
h.started = h.Now()
}
@@ -8,7 +8,6 @@ import (
"sort"
"strings"
"time"
_ "time/tzdata" // the operator's zone is known whatever this machine has installed
sdk "git.novox.be/novox/mesh-sdk/go/delivery"
)
@@ -58,12 +57,6 @@ type checkerVerdict struct {
Machine string `json:"machine,omitempty"`
Words string `json:"words,omitempty"`
} `json:"failed,omitzero"`
// Composition is what the check of the machines' composition made of it (mesh-sdk delivery.Composition): its state
// not-run says the composition, and with it a placement change's answers from its nodes, was not judged.
Composition *struct {
State string `json:"state"`
Words string `json:"words,omitempty"`
} `json:"composition,omitempty"`
}
// CheckerVerdictChanged takes a key of the checker's verdicts bucket (`<group>.<heads hash>`) put or deleted (an empty
@@ -82,13 +75,9 @@ func (h *Holder) CheckerVerdictChanged(key string, value []byte) {
if h.runningSince == nil {
h.runningSince = map[string]time.Time{}
}
if h.verdictGone == nil {
h.verdictGone = map[string]time.Time{}
}
if len(value) == 0 || string(value) == "null" {
delete(h.checker, hash)
delete(h.runningSince, hash)
h.verdictGone[hash] = h.Now()
h.Poke()
return
}
@@ -104,7 +93,6 @@ func (h *Holder) CheckerVerdictChanged(key string, value []byte) {
h.runningSince[v.Heads] = h.Now() // a check starts: a merge it holds says since when (ADR 0548 §2)
} else if v.State != checkerRunning {
delete(h.runningSince, v.Heads)
delete(h.verdictGone, v.Heads)
}
h.checker[v.Heads] = v
h.Poke()
@@ -341,11 +329,11 @@ func (h *Holder) checkAt(members []*Delivery) checkSaid {
if ok {
switch v.State {
case checkerPass:
return checkSaid{Key: checkSaidPass, Words: "passed its check" + notComposed(v, members)}
return checkSaid{Key: checkSaidPass, Words: "passed its check"}
case checkerRunning:
words := "its check has no verdict yet: it is running"
if since, held := h.runningSince[hash]; held && !since.IsZero() {
words += " (since " + clockSaid(since) + ")"
words += " (since " + since.UTC().Format("15:04") + " UTC)"
}
return checkSaid{Key: checkSaidRunning, Words: words}
default:
@@ -383,31 +371,3 @@ func (h *Holder) checkAt(members []*Delivery) checkSaid {
}
return checkSaid{Key: checkSaidNone, Words: "its check has no verdict yet"}
}
// notComposed is what a passing verdict adds when its composition was not judged (mesh-sdk delivery.CompositionNotRun): a
// placement change's pass then holds no node's answer to its placement (ADR 0559 §5), which the ask says. Empty otherwise.
func notComposed(v checkerVerdict, members []*Delivery) string {
if v.Composition == nil || v.Composition.State != sdk.CompositionNotRun {
return ""
}
why := firstOf(oneLine(v.Composition.Words), "it was not run")
for _, m := range members {
for _, p := range m.Paths {
if strings.HasPrefix(p, "placement/") && strings.HasSuffix(p, ".json") {
return ", but its composition was not judged (" + why + "), so no node answered its placement"
}
}
}
return ", but its composition was not judged (" + why + ")"
}
// operatorZone is the zone the operator reads times in, as the messenger says them (its time-zone setting).
var operatorZone = func() *time.Location {
if loc, err := time.LoadLocation("Europe/Brussels"); err == nil {
return loc
}
return time.UTC
}()
// clockSaid is a time of day as the operator reads it: in their zone, named ("00:35 CET").
func clockSaid(t time.Time) string { return t.In(operatorZone).Format("15:04 MST") }
@@ -446,75 +446,9 @@ func (h *Holder) ungatedWaits(d *Delivery) string {
}
}
}
if h.covering(d) == nil {
return h.verdictWaits(d)
}
return ""
}
// askCheckBound is how long an Approve waits for the check's verdict at its heads while the check gates no merge: past it,
// the operator is asked anyway, and the ask says the check has not answered. The checker's own step bound is 30 minutes
// and a check runs several steps; an hour covers a whole check.
const askCheckBound = time.Hour
// waitsCheckBeforeAsking is how the wait for the check's verdict begins, as `show` and the delivery's wait say it.
const waitsCheckBeforeAsking = "waits its check before asking the operator's Approve: "
// verdictWaits is why an Approve is not asked yet while the check gates no merge: the check at the heads the ask would cover
// has no verdict (the operator, 2026-10-27 23:54: "if you don't want me to approve it, don't send the approval request").
// A pass or a failure is asked (a failure in its words: the operator may still merge it), and so is a check that has not
// answered within askCheckBound of the heads becoming ready, which the ask says. For a group the check is the one of all its
// heads together; for a placement change it ends only once every node it places answered (ADR 0559 §5). Under the lock.
func (h *Holder) verdictWaits(d *Delivery) string {
members := h.askMembers(d)
c := h.checkAt(members)
if c.Key == checkSaidPass || c.Key == checkSaidFail {
return ""
}
until := h.waitSince(d, members).Add(askCheckBound)
if !h.Now().Before(until) {
return ""
}
h.lookAt(until, "an Approve that waits its check, asked anyway at its bound")
return waitsCheckBeforeAsking + c.Words + "; asked anyway at " + clockSaid(until) + " if it has not answered"
}
// waitSince is when the wait for a verdict began, the moment the bound of verdictWaits counts from: the latest of the
// heads becoming ready (a member with no ready run yet, such as one re-pushed or settled ready by an event this look has
// not reached, became ready now), the check at those heads starting again, and its verdict being taken away. Under the lock.
func (h *Holder) waitSince(d *Delivery, members []*Delivery) time.Time {
now := h.Now()
since := time.Time{}
later := func(t time.Time) {
if t.After(since) {
since = t
}
}
for _, m := range append([]*Delivery{d}, members...) {
if m.Ready == nil {
return now
}
later(m.Ready.Seen)
}
if hash, err := sdk.HeadsHash(headsOf(members)); err == nil {
later(h.runningSince[hash])
later(h.verdictGone[hash])
}
if since.IsZero() || since.After(now) {
return now
}
return since
}
// unansweredWords is the check's words in an ask made past askCheckBound with no verdict: it says the check has not answered.
func unansweredWords(c checkSaid) checkSaid {
if c.Key == checkSaidPass || c.Key == checkSaidFail {
return c
}
c.Words += ", and has given none in " + strconv.Itoa(int(askCheckBound.Minutes())) + " minutes, so the operator is asked anyway"
return c
}
// waitsForReview is how a hold for a review begins, as the delivery's state and its mesh/delivery status say it.
const waitsForReview = "waits for a review at "
@@ -1,6 +1,7 @@
package main
import (
"encoding/json"
"errors"
"sort"
"strings"
@@ -69,42 +70,27 @@ func TestAnApprovedPullRequestIsMergedThroughTheMergeTool(t *testing.T) {
}
// 1733: the Approve's ask says the check's state at the heads it names, read from the checker's verdict there: "passed
// its check" only when it passed. While the check gates no merge, the ask waits for the check's verdict (the operator,
// 2026-10-27 23:54: "if you don't want me to approve it, don't send the approval request"): a pass or a failure is asked,
// the failure in its words; with no verdict nothing is asked and the delivery says it waits its check, until the bound,
// past which it is asked anyway, saying so, and asked again in new words once the check passes. The verdict is the
// checker's verdicts bucket value as captured on the bus (mesh-delivery-checker, `<group>.<heads hash>`).
// its check" only when it passed. While the check gates no merge, the ask is made before any verdict, says that Approve
// merges before the check ends, and is asked again in new words once the check passes or fails; a running check is no
// new word.
for _, row := range []struct {
name, state, failed, then, path string
gated bool
notComposed bool
after time.Duration
asks, open int
want, not []string
name, state, failed string
gated bool
asks int
want, not []string
}{
{name: "gated, passed", gated: true, state: "pass", asks: 1,
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; asked anyway at 15:00 CEST if"}},
{name: "ungated, running", state: "running", asks: 0, want: []string{waitsCheckBeforeAsking + "its check has no verdict yet: it is running"}},
{name: "ungated, running past its bound", state: "running", after: askCheckBound, asks: 1,
want: []string{"its check has no verdict yet: it is running", "has given none in 60 minutes, so the operator is asked anyway",
"Approve merges it into main at " + readyHead[:12] + " now, before its check ends"}, not: []string{"passed"}},
{name: "ungated, past its bound, then passed", state: "running", after: askCheckBound, then: "pass", asks: 2,
want: []string{"passed its check. Approve merges it into main at " + readyHead[:12] + ";"}, not: []string{"before its check ends", "anyway"}},
{name: "ungated, passed", state: "pass", asks: 1,
{name: "ungated, no verdict", asks: 1,
want: []string{"its check has no verdict yet", "Approve merges it into main at " + readyHead[:12] + " now, before its check ends"},
not: []string{"passed"}},
{name: "ungated, running", state: "running", asks: 1,
want: []string{"its check has no verdict yet", "before its check ends"}, not: []string{"passed"}},
{name: "ungated, then passed", state: "pass", asks: 2,
want: []string{"passed its check. Approve merges it into main at " + readyHead[:12] + ";"}, not: []string{"before its check ends"}},
// A pass that turns back into a check with no verdict (a check started again, its verdict taken away) past the bound
// withdraws the ask, and the bound counts again from that moment: nothing is asked at once as unanswered.
{name: "ungated, passed, running again past its bound", state: "pass", after: askCheckBound, then: "running", asks: 1, open: -1,
want: []string{waitsCheckBeforeAsking + "its check has no verdict yet: it is running (since 15:00 CEST); asked anyway at 16:00 CEST"}},
{name: "ungated, passed, its verdict gone past its bound", state: "pass", after: askCheckBound, then: "gone", asks: 1, open: -1,
want: []string{waitsCheckBeforeAsking + "its check has no verdict yet; asked anyway at 16:00 CEST"}},
// A placement change's pass with its composition not judged holds no node's answer, and the ask says so (ADR 0559 §5).
{name: "ungated, placement passed uncomposed", state: "pass", path: "placement/node-a.json", notComposed: true, asks: 1,
want: []string{"passed its check, but its composition was not judged (the assigner is not running), so no node answered its placement"}},
{name: "ungated, failed", state: "fail", failed: "TestThing: wanted 1, got 2", asks: 1,
want: []string{"its check failed: TestThing: wanted 1, got 2", "although its check failed"}, not: []string{"passed", "anyway"}},
{name: "ungated, then failed", state: "fail", failed: "TestThing: wanted 1, got 2", asks: 2,
want: []string{"its check failed: TestThing: wanted 1, got 2", "although its check failed"}, not: []string{"passed"}},
} {
t.Run(row.name, func(t *testing.T) {
v := newWorld(t)
@@ -112,46 +98,26 @@ func TestAnApprovedPullRequestIsMergedThroughTheMergeTool(t *testing.T) {
ungated(v)
}
v.op.auto = false
p := pr("novox/app", 7, readyHead, "feat/x", firstOf(row.path, "src/a.go"))
p := pr("novox/app", 7, readyHead, "feat/x", "src/a.go")
p.Title = "Fix it"
v.h.PullUpdated(p)
head := headOf("novox/app", 7, readyHead)
put := func() {
hash, _ := sdk.HeadsHash([]sdk.Head{headOf("novox/app", 7, readyHead)})
value, _ := json.Marshal(map[string]any{"v": 1, "group": "g", "heads": hash, "state": row.state,
"failed": map[string]string{"words": row.failed}})
v.h.CheckerVerdictChanged("g."+hash, value)
}
if row.gated {
v.h.Checked(judgedAt("novox/app", 7, readyHead, "pass", "t1t1t1t1t1t1"))
}
v.settleAll()
if !row.gated && row.state != "" {
var more map[string]any
if row.notComposed {
more = map[string]any{"composition": map[string]string{"state": "not-run", "words": "the assigner is not running"}}
}
checkerSaysWith(v, "g", row.state, row.failed, more, head)
v.settleAll()
}
if row.after > 0 {
v.later(row.after)
v.settleAll()
}
if row.then != "" {
checkerSays(v, "g", row.then, row.failed, head)
put()
v.settleAll()
}
open := v.op.open()
wantOpen := min(row.asks, 1)
if row.open < 0 {
wantOpen = 0
}
if v.op.count() != row.asks || len(open) != wantOpen {
t.Fatalf("open asks %v, asked %d times; wanted asked %d times, %d open", open, v.op.count(), row.asks, wantOpen)
}
if wantOpen == 0 {
d := v.delivery(IDOf("novox/app", readyHead))
for _, w := range row.want {
if d.Ready == nil || !strings.Contains(d.Ready.Waits, w) {
t.Fatalf("the delivery's wait does not say %q: %+v", w, d.Ready)
}
}
return
if len(open) != 1 || v.op.count() != row.asks {
t.Fatalf("open asks %v, asked %d times; wanted one open, asked %d times", open, v.op.count(), row.asks)
}
e := v.op.ask(open[0]).Explanation
for _, w := range row.want {
@@ -180,8 +146,8 @@ func TestAnApprovedPullRequestIsMergedThroughTheMergeTool(t *testing.T) {
u.h.PullUpdated(pr("novox/app", 7, readyHead, "feat/x", "src/a.go"))
u.h.PullUpdated(pr("novox/app", 8, other, "feat/y", "src/b.go"))
u.settleAll()
if u.op.count() != 0 || u.state(IDOf("novox/app", readyHead)) != Ready {
t.Fatalf("ungated, two reviewed pull requests with no verdict: %d asks, %s; wanted ready and not asked", u.op.count(), u.state(IDOf("novox/app", readyHead)))
if u.op.count() != 2 || u.state(IDOf("novox/app", readyHead)) != Ready {
t.Fatalf("ungated, two reviewed pull requests with no verdict: %d asks, %s", u.op.count(), u.state(IDOf("novox/app", readyHead)))
}
if gates, _ := u.h.Status("listening")["check-gates-merge"].(bool); gates {
t.Fatalf("delivery_status does not say the check gates no merge: %v", u.h.Status("listening"))
@@ -191,12 +157,8 @@ func TestAnApprovedPullRequestIsMergedThroughTheMergeTool(t *testing.T) {
t.Fatalf("show does not say the check gates no merge: %v", shown)
}
u.h.Checked(judgedAt("novox/app", 8, other, "fail", "t1t1t1t1t1t1"))
checkerSays(u, "feat/y", "fail", "TestB", headOf("novox/app", 8, other))
u.h.PullMerged(merged("novox/app", 7, readyHead, "m7m7m7m7m7m7"))
u.settleAll()
if u.op.count() != 1 {
t.Fatalf("ungated, a failed verdict is asked once (the check gates no merge): %d asks", u.op.count())
}
d = u.delivery(IDOf("novox/app", other))
if d.State != Ready || d.Check == nil || d.Check.Passes() {
t.Fatalf("ungated, after a failed verdict and a trunk's move: %s, verdict %+v", d.State, d.Check)
@@ -361,44 +323,14 @@ func TestAGroupMergesEveryMemberInOnePassOnlyOnceComposedAndApproved(t *testing.
u.h.PullUpdated(pr("novox/mesh-controller", 1, ctlHead, "feat/x", "internal/a.go"))
u.h.PullUpdated(pr("novox/mesh-catalog", 2, catHead, "feat/x", "modules/app/module.json"))
u.settleAll()
group := []sdk.Head{headOf("novox/mesh-controller", 1, ctlHead), headOf("novox/mesh-catalog", 2, catHead)}
checkerSays(u, "feat/x", "running", "", group...)
u.settleAll()
// The group's check is the one of its heads together: while it runs, nothing is asked and each member says it waits.
if d := u.delivery(IDOf("novox/mesh-catalog", catHead)); u.op.count() != 0 || d.Ready == nil ||
!strings.Contains(d.Ready.Waits, waitsCheckBeforeAsking+"its check has no verdict yet: it is running") {
t.Fatalf("ungated, the group is asked %d times while its check runs; wanted none, and the wait said: %+v", u.op.count(), d.Ready)
}
checkerSays(u, "feat/x", "pass", "", group...)
u.settleAll()
if u.op.count() != 1 {
t.Fatalf("ungated, the group is asked %d times once its check passed; wanted once", u.op.count())
t.Fatalf("ungated, the group is asked %d times; wanted once", u.op.count())
}
if e := u.op.ask(u.op.open()[0]).Explanation; !strings.Contains(e, "2 pull requests, passed its check. Approve merges all of them into main in one pass, at the heads named;") {
t.Fatalf("ungated, the group's ask does not say its check passed:\n%s", e)
// 1733: the group's ask claims no pass it has not had.
if e := u.op.ask(u.op.open()[0]).Explanation; strings.Contains(e, "passed") ||
!strings.Contains(e, "2 pull requests: its check has no verdict yet. Approve merges all of them into main in one pass, at the heads named, now, before its check ends;") {
t.Fatalf("ungated, the group's ask does not say its check has no verdict yet:\n%s", e)
}
// A member re-pushed past the bound after its sibling became ready: the bound counts from the new head, so nothing is
// asked as unanswered at once (review round 1).
r := newWorld(t)
ungated(r)
r.op.auto = false
const ctlHead2 = "c3c3c3c3c3c3c3c3c3c3c3c3c3c3c3c3c3c3c3c3"
r.h.PullUpdated(pr("novox/mesh-controller", 1, ctlHead, "feat/x", "internal/a.go"))
r.h.PullUpdated(pr("novox/mesh-catalog", 2, catHead, "feat/x", "modules/app/module.json"))
r.settleAll()
r.later(askCheckBound + time.Minute)
r.h.PullUpdated(pr("novox/mesh-controller", 1, ctlHead2, "feat/x", "internal/a.go"))
// Made ready by a handler, before the look gives it a run: no verdict and no status at the new heads, so only the rule
// that a member with no run counts as ready now keeps the older sibling's time from asking it as unanswered.
r.h.mu.Lock()
r.h.ungate(r.h.deliveries[IDOf("novox/mesh-controller", ctlHead2)], r.h.Now())
r.h.mu.Unlock()
r.settleAll()
if d := r.delivery(IDOf("novox/mesh-catalog", catHead)); r.op.count() != 0 || d.Ready == nil ||
!strings.Contains(d.Ready.Waits, waitsCheckBeforeAsking) {
t.Fatalf("a member re-pushed past the bound: asked %d times, wait %+v; wanted none asked and the wait said", r.op.count(), d.Ready)
}
u.op.answer(u.op.open()[0], OptionApprove)
u.settleAll()
if len(u.forge.merges) != 2 {
+1 -1
View File
@@ -99,7 +99,7 @@
},
"check-gates-merge": {
"type": "boolean",
"description": "Whether mesh/check gates the merge. True (the default): the operator's Approve is asked, and a merge handed, only on a passing verdict judged against the trunk's head, and a trunk's move puts a verdict out of date (ADR 0447, ADR 0505 §4). False (the operator, 2026-10-25 22:56: \"run the check locally only, but not as a gate, it loses us too much time\"; developers run the unit and integration levels locally before a pull request): a pull request reviewed at its head is ready, and its Approve is asked once the check at its heads (a group: of all its heads together) has a verdict, a pass or a failure said in the ask, or after 60 minutes with none, said so (the operator, 2026-10-27 23:54: \"if you don't want me to approve it, don't send the approval request\"); an approved merge waits for no verdict; no merge is refused for a check judged on an older trunk; a later verdict is shown on the pull request and the delivery, and asks a pending Approve again in its words; a group still merges all its members in one pass. Set it false together with the gitea module's check-gates-merge, which lets its merge tool merge an approved head without a passing mesh/check.",
"description": "Whether mesh/check gates the merge. True (the default): the operator's Approve is asked, and a merge handed, only on a passing verdict judged against the trunk's head, and a trunk's move puts a verdict out of date (ADR 0447, ADR 0505 §4). False (the operator, 2026-10-25 22:56: \"run the check locally only, but not as a gate, it loses us too much time\"; developers run the unit and integration levels locally before a pull request): a pull request reviewed at its head is ready and its Approve asked at once, with no verdict waited on; no merge is refused for a check judged on an older trunk; a verdict that comes is shown on the pull request and the delivery, never waited on; a group still merges all its members in one pass. Set it false together with the gitea module's check-gates-merge, which lets its merge tool merge an approved head without a passing mesh/check.",
"trusted": false,
"default": true
}