From 2d1d4bf988822c5edaea8ba18c9e3c7cc00ee903 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 9 Oct 2026 02:24:26 +0200 Subject: [PATCH 1/3] End a merge that moves nothing or that a later merge delivered, and lift a group hold its check answered (hq issues 319, 316) A held delivery asked a person a question with no right answer: four hq merges that build nothing, merges adding a module the controller sends nowhere, controller merges whose change a later walk of the same trunk already delivered, and a group member held because its composed check had not yet answered. Only stop could end them, which marks a delivered change as an error on its pull request. - A merge whose plan moves no module (nothing, or only a new module sent nowhere) is delivered at once. - A new final state, covered: every module a merge moves was sent by a finished walk of a later merge into the same trunk, which contains it. Shown as success on the pull request; a group counts it as delivered. - A group member waits published while its composed check is asked and unanswered, and a member held for it is lifted back to published once it passes for the heads that merged. Read again each tick, so the deliveries already held end by themselves after the rollout. --- .../cmd/mesh-delivery/covered_test.go | 407 ++++++++++++++++++ .../cmd/mesh-delivery/effects.go | 2 + .../cmd/mesh-delivery/fakes_test.go | 6 + .../mesh-delivery/cmd/mesh-delivery/holder.go | 116 ++++- .../mesh-delivery/cmd/mesh-delivery/model.go | 19 +- .../mesh-delivery/cmd/mesh-delivery/table.go | 56 ++- .../cmd/mesh-delivery/table_test.go | 24 +- 7 files changed, 614 insertions(+), 16 deletions(-) create mode 100644 modules/mesh-delivery/cmd/mesh-delivery/covered_test.go diff --git a/modules/mesh-delivery/cmd/mesh-delivery/covered_test.go b/modules/mesh-delivery/cmd/mesh-delivery/covered_test.go new file mode 100644 index 00000000..62686249 --- /dev/null +++ b/modules/mesh-delivery/cmd/mesh-delivery/covered_test.go @@ -0,0 +1,407 @@ +package main + +import ( + "fmt" + "strings" + "testing" + "time" +) + +// novox/hq issues 319 and 316: a held delivery whose change already runs, or that builds nothing, is not a +// question for a person. A merge that moves nothing is delivered at once; a merge whose every module a later +// merge on the same trunk delivered is covered — never stopped, never an error on its pull request; and a +// group member held for a composed check that had not answered goes on once it passes. + +var t0 = time.Date(2026, 10, 6, 12, 0, 0, 0, time.UTC) + +// mergedAt is the forge's `pull.merged` at a moment. +func mergedAt(p PullEvent, merge string, at time.Time) PullEvent { + p.MergeCommit = merge + p.MergedAt = at.UTC().Format(time.RFC3339) + return p +} + +// verdictMoving is a passing verdict whose plan moves these modules — or builds nothing, given none. +func verdictMoving(repo string, number int, sha string, modules ...string) CheckedEvent { + c := verdict(repo, number, sha, "pass") + c.Plan = planMoving(repo, modules...) + return c +} + +func planMoving(repo string, modules ...string) *DeliveryPlan { + if len(modules) == 0 { + return &DeliveryPlan{Repository: repo, Base: "main", Summary: "builds nothing: the change touches no module of the mesh's graph"} + } + return &DeliveryPlan{Repository: repo, Base: "main", Moved: modules, Summary: "builds " + strings.Join(modules, ", ") + " → anchor"} +} + +// deliverLater is a later merge into a trunk, walked: its walk opened for its merge commit, let go, and left +// in the state given, every module of it sent. +func deliverLater(w *world, repo string, number int, base, headSHA, merge string, at time.Time, state string, + modules ...string) string { + w.t.Helper() + p := pr(repo, number, headSHA, fmt.Sprintf("feat/later-%d", number)) + p.Base = base + w.h.PullUpdated(p) + w.h.Checked(verdictMoving(repo, number, headSHA, modules...)) + w.h.PullMerged(mergedAt(p, merge, at)) + walk := aWalk("plan-"+merge[:6], repo, merge, true) + walk.Branch = base + walk.Modules = map[string]*WalkModule{} + for _, m := range modules { + walk.Modules[m] = &WalkModule{} + } + w.h.WalkMoved(w.ctl.put(walk)) + w.settleAll() + walk = w.ctl.walk(walk.ID) + sent := at.Add(5 * time.Minute) + for _, m := range modules { + walk.Modules[m] = &WalkModule{State: "built", SentAt: &sent} + } + walk.State = state + w.h.WalkMoved(w.ctl.put(walk)) + w.settleAll() + return IDOf(repo, headSHA) +} + +func statusOn(w *world, repo, commit string) string { + w.forge.mu.Lock() + defer w.forge.mu.Unlock() + return w.forge.statuses[repo+"@"+commit+" mesh/delivery"] +} + +// **A merge that moves nothing is delivered at once** (issue 319): there is nothing for a walk to move and +// nothing for a person to decide — whether its check passed, failed or never answered, and whether its plan +// builds nothing at all or only a new module the controller builds and sends nowhere (issue 300). +func TestAMergeThatMovesNothingIsDeliveredAtOnce(t *testing.T) { + for _, c := range []struct { + name string + verdict string // "" is a merge before any verdict + plan *DeliveryPlan + }{ + {"builds nothing, merged before its check answered", "", planMoving("novox/hq")}, + {"builds nothing, merged on a failed check", "fail", planMoving("novox/hq")}, + {"builds nothing, merged on a passing check", "pass", planMoving("novox/hq")}, + {"a new module, sent nowhere", "pass", &DeliveryPlan{Repository: "novox/hq", New: []string{"modules/zfs"}, + Summary: "builds new: modules/zfs, sent nowhere; no bus step"}}, + {"a new module, sent nowhere, merged before its check answered", "", &DeliveryPlan{Repository: "novox/hq", + New: []string{"modules/zfs"}, Summary: "builds new: modules/zfs, sent nowhere; no bus step"}}, + } { + t.Run(c.name, func(t *testing.T) { + w := newWorld(t) + w.ctl.planned = c.plan + p := pr("novox/hq", 7, head, "docs/x", "02-DECISIONS/x.md") + w.h.PullUpdated(p) + if c.verdict != "" { + v := verdict("novox/hq", 7, head, c.verdict) + v.Plan = c.plan + w.h.Checked(v) + } + w.later(31 * time.Minute) + w.h.PullMerged(mergedAt(p, merge, t0.Add(30*time.Minute))) + w.settleAll() + // Past the bound a merge waits for its walk: nothing holds it then either. + w.later(15 * time.Minute) + if err := w.h.Reconcile(); err != nil { + t.Fatal(err) + } + w.settleAll() + d := w.delivery(IDOf("novox/hq", head)) + if d.State != Delivered { + t.Fatalf("a merge whose plan is %q is %s (%s), not delivered", c.plan.Summary, d.State, d.HeldWhy) + } + if s := statusOn(w, "novox/hq", head); !strings.HasPrefix(s, "success") { + t.Fatalf("its pull request's mesh/delivery says %q", s) + } + }) + } +} + +// **A held delivery whose every module a later merge on its trunk delivered is covered** (issue 319): the +// later merge commit contains it, its walk is done, and it sent each module this one moves. Anything short +// of that — a module not carried, an earlier merge, another trunk, a walk that did not finish — leaves it held +// for a person, as before. +func TestAMergeALaterDeliveredWalkCarriedIsCovered(t *testing.T) { + const ctl = "novox/mesh-controller" + for _, c := range []struct { + name string + // The later merge: its trunk, minutes after t0 it merged (this one merged at 30), its walk's end, and + // what it sent. + base string + at int + state string + modules []string + // Whether this one was held for a person before the later merge was delivered. + heldFirst bool + want State + }{ + {"carried every module, delivered after this one was held", "main", 35, walkDone, + []string{"build-agent", "mesh-controller"}, true, Covered}, + {"carried every module, delivered before this one was due to be held", "main", 35, walkDone, + []string{"build-agent", "mesh-controller", "route-proxy"}, false, Covered}, + {"carried one of its two modules", "main", 35, walkDone, []string{"mesh-controller"}, true, Held}, + {"merged before this one", "main", 25, walkDone, []string{"build-agent", "mesh-controller"}, true, Held}, + {"merged into another branch", "release", 35, walkDone, []string{"build-agent", "mesh-controller"}, true, Held}, + {"its walk failed", "main", 35, walkFailed, []string{"build-agent", "mesh-controller"}, true, Held}, + } { + t.Run(c.name, func(t *testing.T) { + w := newWorld(t) + p := pr(ctl, 1, head, "feat/one", "cmd/a.go") + w.h.PullUpdated(p) + w.h.Checked(verdictMoving(ctl, 1, head, "mesh-controller", "build-agent")) + w.later(31 * time.Minute) + w.h.PullMerged(mergedAt(p, merge, t0.Add(30*time.Minute))) + w.settleAll() + if c.heldFirst { + w.later(15 * time.Minute) + if err := w.h.Reconcile(); err != nil { + t.Fatal(err) + } + w.settleAll() + if s := w.state(IDOf(ctl, head)); s != Held { + t.Fatalf("a merge no walk opened for is %s before the later one", s) + } + } + later := deliverLater(w, ctl, 2, c.base, "bbbbbbbbbbbb2222", "nnnnnnnnnnnn8888", t0.Add(time.Duration(c.at)*time.Minute), + c.state, c.modules...) + if c.state == walkDone && w.state(later) != Delivered { + t.Fatalf("the later merge is %s", w.state(later)) + } + w.later(20 * time.Minute) + if err := w.h.Reconcile(); err != nil { + t.Fatal(err) + } + w.settleAll() + d := w.delivery(IDOf(ctl, head)) + if d.State != c.want { + t.Fatalf("it is %s (%s), want %s", d.State, d.HeldWhy, c.want) + } + s := statusOn(w, ctl, head) + if c.want == Covered { + if !strings.HasPrefix(s, "success covered") || !strings.Contains(s, "nnnnnnnn") { + t.Fatalf("its pull request's mesh/delivery says %q", s) + } + last := d.Transitions[len(d.Transitions)-1] + if !strings.Contains(last.Why, IDOf(ctl, "bbbbbbbbbbbb2222")) { + t.Fatalf("its transition does not name what covered it: %q", last.Why) + } + } + if strings.HasPrefix(s, "error") { + t.Fatalf("its pull request's mesh/delivery is an error: %q", s) + } + }) + } +} + +// A covered member of a group counts as delivered: the member after it is let go, not stopped. +func TestACoveredGroupMemberLetsTheNextOneGo(t *testing.T) { + w := newWorld(t) + const ctl, lab = "novox/mesh-controller", "novox/mesh-lab" + const ctlHead, labHead = "c1c1c1c1c1c1c1c1", "d2d2d2d2d2d2d2d2" + ctlID, labID := IDOf(ctl, ctlHead), IDOf(lab, labHead) + w.ctl.order = func(ms []Member) Order { return Order{Order: []string{ctlID, labID}} } + pc, pl := pr(ctl, 1, ctlHead, "feat/g", "cmd/a.go"), pr(lab, 2, labHead, "feat/g", "replays/a.go") + w.h.PullUpdated(pc) + w.h.PullUpdated(pl) + w.h.Checked(verdictMoving(ctl, 1, ctlHead, "mesh-controller")) + w.h.Checked(verdictMoving(lab, 2, labHead, "mesh-lab")) + w.settleAll() + w.h.Checked(CheckedEvent{Group: "feat/g", ID: "check-1", Verdict: "pass", Summary: "both compose"}) + w.later(31 * time.Minute) + w.h.PullMerged(mergedAt(pc, "m1m1m1m1m1m1", t0.Add(30*time.Minute))) + w.h.PullMerged(mergedAt(pl, "m2m2m2m2m2m2", t0.Add(31*time.Minute))) + w.h.WalkMoved(w.ctl.put(aWalk("plan-lab", lab, "m2m2m2m2m2m2", true))) + w.settleAll() + deliverLater(w, ctl, 3, "main", "e3e3e3e3e3e3e3e3", "m3m3m3m3m3m3", t0.Add(35*time.Minute), walkDone, "mesh-controller") + w.later(20 * time.Minute) + if err := w.h.Reconcile(); err != nil { + t.Fatal(err) + } + w.settleAll() + if w.state(ctlID) != Covered { + t.Fatalf("the controller's member is %s", w.state(ctlID)) + } + if w.state(labID) != Delivering || !containsString(w.ctl.delivers, "plan-lab") { + t.Fatalf("the member after it is %s, let go %v", w.state(labID), w.ctl.delivers) + } + lw := w.ctl.walk("plan-lab") + lw.State = walkDone + w.h.WalkMoved(w.ctl.put(lw)) + w.settleAll() + if state, why := GroupState(w.h.groups["feat/g"], w.h.membersOf(w.h.groups["feat/g"])); state != "delivered" { + t.Fatalf("the group is %s: %s", state, why) + } +} + +// **On rollout, the deliveries already held close by themselves** (issue 319's held ones), and a restart +// after that moves nothing again: what was read back is where it stays. +func TestHeldDeliveriesReadBackCloseByThemselvesOnce(t *testing.T) { + const ctl, cat = "novox/mesh-controller", "novox/mesh-catalog" + merged1, merged2 := t0, t0.Add(2*time.Hour) + coverer := func(repo, module string) *Delivery { + return &Delivery{ID: IDOf(repo, "c0c0c0c0c0c0c0c0"), Repository: repo, Commit: "c0c0c0c0c0c0c0c0", Number: 90, + Base: "main", State: Delivered, Since: merged2.Add(time.Hour), Created: merged2.Add(-time.Hour), + MergedAs: "f0f0f0f0f0f0f0f0", MergedAt: merged2, Plan: planMoving(repo, module), + Walk: &WalkSeen{ID: "plan-90", Commit: "f0f0f0f0f0f0f0f0", State: walkDone}, + Steps: []Step{{Machine: "anchor", Module: module, State: StepPassed}, {Machine: "the rest running it", Module: module, State: StepSent}}} + } + held := func(repo string, plan *DeliveryPlan, why string) *Delivery { + return &Delivery{ID: IDOf(repo, head), Repository: repo, Commit: head, Number: 7, Base: "main", State: Held, + Since: merged1.Add(10 * time.Minute), Created: merged1.Add(-time.Hour), MergedAs: merge, MergedAt: merged1, + HeldWhy: why, Plan: plan, Transitions: []Transition{{At: merged1, From: Ready, To: Held, Event: EvHold, Why: why}}} + } + const unwalkedWhy = "merged, and the controller opened no walk for it within 10m0s — nothing it holds follows that branch, " + + "or the merge was not heard: a person decides" + for _, c := range []struct { + name string + kept []*Delivery + group *Group + want State + }{ + {"merged without a passing check, builds nothing", + []*Delivery{held("novox/hq", planMoving("novox/hq"), "merged without a passing check: only a person decides that it goes on")}, + nil, Delivered}, + {"a new module sent nowhere, no walk opened", + []*Delivery{held(cat, &DeliveryPlan{New: []string{"modules/zfs"}, Summary: "builds new: modules/zfs, sent nowhere"}, unwalkedWhy)}, + nil, Delivered}, + {"no walk opened, a later delivered walk carried its module", + []*Delivery{held(ctl, planMoving(ctl, "mesh-controller"), unwalkedWhy), coverer(ctl, "mesh-controller")}, nil, Covered}, + {"no walk opened, a later delivered walk carried another module", + []*Delivery{held(ctl, planMoving(ctl, "mesh-controller"), unwalkedWhy), coverer(ctl, "route-proxy")}, nil, Held}, + {"held for its group, its own walk superseded, a later delivered walk carried its module", + func() []*Delivery { + d := held(cat, planMoving(cat, "systemd"), "its group feat/j's composed check did not pass for the heads that "+ + "merged; a person decides that it goes on") + d.Group = "feat/j" + d.Walk = &WalkSeen{ID: "plan-7", Commit: merge, State: walkSuperseded, Waits: true, WaitedFor: "mesh-delivery"} + other := &Delivery{ID: IDOf("novox/mesh-host", "abababababababab"), Repository: "novox/mesh-host", + Commit: "abababababababab", Number: 3, Base: "main", Group: "feat/j", State: Delivered, + MergedAs: "acacacacacacacac", MergedAt: merged1} + return []*Delivery{d, other, coverer(cat, "systemd")} + }(), + &Group{ID: "feat/j", Members: []string{IDOf(cat, head), IDOf("novox/mesh-host", "abababababababab")}, Closed: true, + Order: []string{IDOf(cat, head), IDOf("novox/mesh-host", "abababababababab")}, + Check: &GroupCheck{Asked: "check-1", Verdict: "pass", Commits: map[string]string{IDOf(cat, head): head, + IDOf("novox/mesh-host", "abababababababab"): "abababababababab"}}}, + Covered}, + } { + t.Run(c.name, func(t *testing.T) { + store := newMemStore() + for _, d := range c.kept { + if err := store.PutDelivery(d); err != nil { + t.Fatal(err) + } + } + if c.group != nil { + if err := store.PutGroup(c.group); err != nil { + t.Fatal(err) + } + } + now := merged2.Add(30 * time.Hour) + w := newWorldOver(t, store, newFakeController(), newFakeForge(), now) + if err := w.h.Reconcile(); err != nil { + t.Fatal(err) + } + w.settleAll() + id := c.kept[0].ID + d := w.delivery(id) + if d.State != c.want { + t.Fatalf("read back held, it is %s, want %s", d.State, c.want) + } + if s := statusOn(w, d.Repository, d.Commit); c.want != Held && !strings.HasPrefix(s, "success") { + t.Fatalf("its pull request's mesh/delivery says %q", s) + } + // A restart over what it kept: nothing moves again. + again := newWorldOver(t, store, newFakeController(), newFakeForge(), now.Add(time.Hour)) + if err := again.h.Reconcile(); err != nil { + t.Fatal(err) + } + again.settleAll() + if d2 := again.delivery(id); d2.State != d.State || len(d2.Transitions) != len(d.Transitions) { + t.Fatalf("a restart moved it again: %s with %d transitions, was %s with %d", d2.State, len(d2.Transitions), + d.State, len(d.Transitions)) + } + }) + } +} + +// **A group member is not held for a composed check that has not answered** (issue 316): it waits published, +// and goes on once the check passes for the heads that merged. +func TestAGroupMemberWaitsForItsComposedCheckToAnswer(t *testing.T) { + w := newWorld(t) + const a, b = "novox/a", "novox/b" + const aHead, bHead = "a1a1a1a1a1a1a1a1", "b2b2b2b2b2b2b2b2" + aID := IDOf(a, aHead) + pa, pb := pr(a, 1, aHead, "feat/k", "x"), pr(b, 2, bHead, "feat/k", "y") + w.h.PullUpdated(pa) + w.h.PullUpdated(pb) + w.h.Checked(verdictMoving(a, 1, aHead, "a")) + w.h.Checked(verdictMoving(b, 2, bHead, "b")) + w.settleAll() + if len(w.ctl.checks) != 1 { + t.Fatalf("the composed check was asked %v", w.ctl.checks) + } + w.h.PullMerged(mergedAt(pa, "m1m1m1m1m1m1", t0)) + w.h.WalkMoved(w.ctl.put(aWalk("plan-a", a, "m1m1m1m1m1m1", true))) + w.settleAll() + if s := w.state(aID); s != Published { + t.Fatalf("merged while its group's composed check had not answered, it is %s (%s)", s, w.delivery(aID).HeldWhy) + } + w.h.Checked(CheckedEvent{Group: "feat/k", ID: "check-1", Verdict: "pass", Summary: "both compose"}) + w.settleAll() + if s := w.state(aID); s != Delivering || !containsString(w.ctl.delivers, "plan-a") { + t.Fatalf("its composed check passed, and it is %s, let go %v", s, w.ctl.delivers) + } +} + +// **A member already held for its group's composed check goes on when that check passes for the heads that +// merged** (issue 316); a check that failed, or passed for other heads, keeps it held for a person. +func TestAMemberHeldForItsComposedCheckIsLiftedWhenItPasses(t *testing.T) { + const a, b = "novox/a", "novox/b" + const aHead, bHead = "a1a1a1a1a1a1a1a1", "b2b2b2b2b2b2b2b2" + aID, bID := IDOf(a, aHead), IDOf(b, bHead) + for _, c := range []struct { + name string + verdict string + commits map[string]string + want State + }{ + {"passed for the heads that merged", "pass", map[string]string{aID: aHead, bID: bHead}, Delivering}, + {"warned for the heads that merged", "warning", map[string]string{aID: aHead, bID: bHead}, Delivering}, + {"failed", "fail", map[string]string{aID: aHead, bID: bHead}, Held}, + {"passed for other heads", "pass", map[string]string{aID: "a9a9a9a9a9a9a9a9", bID: bHead}, Held}, + } { + t.Run(c.name, func(t *testing.T) { + store := newMemStore() + why := "its group feat/k's composed check did not pass for the heads that merged; a person decides that it goes on" + da := &Delivery{ID: aID, Repository: a, Commit: aHead, Number: 1, Base: "main", Branch: "feat/k", Group: "feat/k", + State: Held, Since: t0, Created: t0.Add(-time.Hour), MergedAs: "m1m1m1m1m1m1", MergedAt: t0, HeldWhy: why, + Plan: planMoving(a, "a"), Check: &Verdict{Gate: "pass"}, + Walk: &WalkSeen{ID: "plan-a", Commit: "m1m1m1m1m1m1", State: walkBuilding, Waits: true, WaitedFor: "mesh-delivery"}} + db := &Delivery{ID: bID, Repository: b, Commit: bHead, Number: 2, Base: "main", Branch: "feat/k", Group: "feat/k", + State: Ready, Since: t0, Created: t0.Add(-time.Hour), Plan: planMoving(b, "b"), Check: &Verdict{Gate: "pass"}} + g := &Group{ID: "feat/k", Members: []string{aID, bID}, Order: []string{aID, bID}, Closed: true, + OrderOf: map[string]string{aID: aHead, bID: bHead}, + Check: &GroupCheck{Asked: "check-1", At: t0, Commits: c.commits}} + for _, d := range []*Delivery{da, db} { + _ = store.PutDelivery(d) + } + _ = store.PutGroup(g) + ctl := newFakeController() + ctl.put(aWalk("plan-a", a, "m1m1m1m1m1m1", true)) + w := newWorldOver(t, store, ctl, newFakeForge(), t0.Add(time.Minute)) + w.settleAll() + if s := w.state(aID); s != Held { + t.Fatalf("before its composed verdict it is %s", s) + } + w.h.Checked(CheckedEvent{Group: "feat/k", ID: "check-1", Verdict: c.verdict, Summary: "composed: " + c.verdict}) + w.settleAll() + if s := w.state(aID); s != c.want { + t.Fatalf("its composed check %s, and it is %s (%s), want %s", c.name, s, w.delivery(aID).HeldWhy, c.want) + } + if c.want == Delivering && !containsString(ctl.delivers, "plan-a") { + t.Fatalf("lifted, its walk was not let go: %v", ctl.delivers) + } + }) + } +} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/effects.go b/modules/mesh-delivery/cmd/mesh-delivery/effects.go index b2522fe8..83fb8c84 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/effects.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/effects.go @@ -107,6 +107,8 @@ func statusOf(d *Delivery) (string, string) { return "error", clip("stopped: "+why, 140) case Retired: return "success", "history: merged before this owner delivered anything, never walked" + case Covered: + return "success", clip("covered: "+d.CoveredBy, 140) } return "pending", string(d.State) } diff --git a/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go b/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go index 73506256..1ecacccf 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go @@ -95,6 +95,8 @@ type fakeController struct { down bool // onCheck runs as a check is asked, before it is answered: what the forge held at that moment. onCheck func() + // planned is what Plan answers instead of "builds app", when set. + planned *DeliveryPlan } func newFakeController() *fakeController { @@ -108,6 +110,10 @@ func (c *fakeController) Plan(repository, base, head string, paths, dirs, remove if c.down { return nil, errors.New("down") } + if c.planned != nil { + p := *c.planned + return &p, nil + } return &DeliveryPlan{Repository: repository, Base: base, Head: head, Moved: []string{"app"}, Summary: "builds app"}, nil } func (c *fakeController) Order(ms []Member) (Order, error) { diff --git a/modules/mesh-delivery/cmd/mesh-delivery/holder.go b/modules/mesh-delivery/cmd/mesh-delivery/holder.go index 84e96b68..a5114dce 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/holder.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/holder.go @@ -160,7 +160,7 @@ func (d *Delivery) owe(e Effect) { func afterTheTrunk(s State) bool { return s == Published || s == Delivering || s == Held || s == Delivered || s == Failed || s == Stopped || - s == Superseded || s == Retired + s == Superseded || s == Retired || s == Covered } func orNothing(s State) string { @@ -755,10 +755,26 @@ func (h *Holder) plan() []func() { } } + // Held, or merged with no walk opened for it: ended by what is already true of it, never asked of a person + // (novox/hq issue 319). A plan that moves nothing is delivered; one whose every module a later merge into the + // same trunk delivered is covered. Read again each tick, so a delivery read back after a restart, or held + // before this was known, ends by itself. + for _, d := range h.deliveries { + if d.State != Held && !h.unwalkedPast(d, now) { + continue + } + if d.CoveredBy == "" { + d.CoveredBy = h.coverOf(d) + } + if ts := settle(d, Facts{Now: now}); len(ts) > 0 { + h.moved(d, ts) + h.closeGroupOf(d) + } + } + // Merged, and no walk opened for it after the walks were read again: never silent — held, saying so. for _, d := range h.deliveries { - if d.State == Ready && d.MergedAs != "" && d.Walk == nil && !d.MovesNothing() && d.HeldWhy == "" && - now.Sub(d.MergedAt) > unwalked && h.reconciled.After(d.MergedAt.Add(unwalked)) { + if h.unwalkedPast(d, now) { d.HeldWhy = "merged, and the controller opened no walk for it within " + unwalked.String() + " — nothing it " + "holds follows that branch, or the merge was not heard: a person decides" h.settleAndKeep(d) @@ -796,6 +812,74 @@ func (h *Holder) plan() []func() { return asks } +// unwalkedPast is whether a merged delivery has gone without a walk past the bound one opens within, read +// again after it: what holds it for a person, unless something already ends it. +func (h *Holder) unwalkedPast(d *Delivery, now time.Time) bool { + return d.State == Ready && d.MergedAs != "" && d.Walk == nil && !d.MovesNothing() && d.HeldWhy == "" && + now.Sub(d.MergedAt) > unwalked && h.reconciled.After(d.MergedAt.Add(unwalked)) +} + +// coverOf says which later merges cover a merged delivery, or nothing (novox/hq issue 319): for every module +// its plan moves, a delivered delivery of the same repository and trunk, merged after it — so its merge commit +// contains this one's, the trunk being protected and only ever merged into — whose finished walk sent that +// module. Read from this owner's own records: the forge's merge times and the walks' records. +func (h *Holder) coverOf(d *Delivery) string { + if d.MergedAs == "" || d.MergedAt.IsZero() || d.Base == "" || d.Plan == nil || len(d.Plan.Moved) == 0 { + return "" + } + carried := map[string][]string{} + var by []*Delivery + for _, module := range d.Plan.Moved { + var first *Delivery + for _, c := range h.deliveries { + if carries(c, d, module) && (first == nil || c.MergedAt.Before(first.MergedAt) || + c.MergedAt.Equal(first.MergedAt) && c.ID < first.ID) { + first = c + } + } + if first == nil { + return "" + } + if carried[first.ID] == nil { + by = append(by, first) + } + carried[first.ID] = append(carried[first.ID], module) + } + sort.Slice(by, func(i, j int) bool { return by[i].MergedAt.Before(by[j].MergedAt) }) + var parts []string + for _, c := range by { + mods := carried[c.ID] + sort.Strings(mods) + parts = append(parts, fmt.Sprintf("delivered with %s (merged as %s, walk %s): %s", c.ID, shortOf(c.MergedAs, 8), + c.Walk.ID, strings.Join(mods, ", "))) + } + return fmt.Sprintf("%s — merged into %s after %s, which it therefore contains", strings.Join(parts, "; "), d.Base, + shortOf(d.MergedAs, 8)) +} + +// carries is whether c delivered a module after d, from a commit that contains d's merge. +func carries(c, d *Delivery, module string) bool { + if c.ID == d.ID || c.State != Delivered || !strings.EqualFold(c.Repository, d.Repository) || + !strings.EqualFold(c.Base, d.Base) || c.MergedAs == "" || sameCommit(c.MergedAs, d.MergedAs) || + !c.MergedAt.After(d.MergedAt) || c.Walk == nil || c.Walk.State != walkDone { + return false + } + if containsString(c.Walk.Sent, module) { + return true + } + for _, s := range c.Steps { + if s.Module == module && (s.State == StepPassed || s.State == StepSent) { + return true + } + } + return false +} + +// heldForGroup is whether a member is held only because its group's composed check had not passed. +func heldForGroup(d *Delivery, g *Group) bool { + return d.State == Held && strings.HasPrefix(d.HeldWhy, fmt.Sprintf("its group %s's composed check", g.ID)) +} + // askCheck is the ask of a proposed delivery's own check, when nothing else will bring its verdict: no verdict, // no check this owner asked, and proposed past the grace the controller's own check of its announcement is // given. Nil otherwise — once asked, a late verdict is the bound's and H2's to speak for. @@ -1031,7 +1115,7 @@ func (h *Holder) groupTurns(g *Group, members []*Delivery) []func() { for i, m := range ordered { d := byID[m.ID] switch { - case d.State == Delivered || d.State == Retired: + case d.State.Reached(): continue case d.State == Failed || d.State == Stopped || d.State == Superseded: var asks []func() @@ -1041,7 +1125,27 @@ func (h *Holder) groupTurns(g *Group, members []*Delivery) []func() { } } return asks + case d.State == Held: + // Held for a composed check that has since passed for the heads that merged: lifted, published again, + // and its turn taken in order (novox/hq issue 316). Any other hold is a person's. + if composed && heldForGroup(d, g) && walkCanGoOn(d) { + d.HeldWhy = "" + d.Lifted = fmt.Sprintf("its group %s's composed check said %s for the heads that merged: %s", g.ID, + g.Check.Verdict, g.Check.Summary) + h.settleAndKeep(d) + if d.State == Published { + if a := h.turn(d, fmt.Sprintf("its turn in group %s (%d of %d)", g.ID, i+1, len(ordered))); a != nil { + return []func(){a} + } + } + } + return nil case d.State == Published: + // Asked and not answered is not "did not pass": it waits, published, for the answer (issue 316). + if !composed && d.HeldWhy == "" && g.Check != nil && g.Check.Verdict == "" && + sameMembers(g.Check.Commits, commitsOf(members)) { + return nil + } if !composed && d.HeldWhy == "" { d.HeldWhy = fmt.Sprintf("its group %s's composed check did not pass for the heads that merged; a person "+ "decides that it goes on", g.ID) @@ -1093,10 +1197,10 @@ func GroupState(g *Group, members []*Delivery) (string, string) { return "failed", "a member failed" case count[Stopped] > 0 && g.Closed: return "stopped", "a member was stopped" - case n > 0 && count[Delivered]+count[Retired] == n: + case n > 0 && count[Delivered]+count[Retired]+count[Covered] == n: return "delivered", "every member is delivered" case g.Closed: - return "delivering", fmt.Sprintf("%d of %d delivered", count[Delivered], n) + return "delivering", fmt.Sprintf("%d of %d delivered", count[Delivered]+count[Covered], n) case len(g.Cycle) > 0: return "rejected", cycleSaid(g) case count[Rejected] > 0: diff --git a/modules/mesh-delivery/cmd/mesh-delivery/model.go b/modules/mesh-delivery/cmd/mesh-delivery/model.go index c778cbed..fa98d32f 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/model.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/model.go @@ -61,6 +61,10 @@ type Delivery struct { // History is why it is history, once it is (novox/hq issue 313). FromForge bool `json:"from_forge,omitempty"` History string `json:"history,omitempty"` + // CoveredBy says which later merges into the same trunk delivered every module it moves, once they did + // (novox/hq issue 319); Lifted why what held it for a person no longer holds (issue 316). + CoveredBy string `json:"covered_by,omitempty"` + Lifted string `json:"lifted,omitempty"` // Walk is the controller's walk of its trunk commit, as last read; Steps the machines it reached. Walk *WalkSeen `json:"walk,omitempty"` Steps []Step `json:"steps,omitempty"` @@ -144,10 +148,11 @@ type MachinePlan struct { Waits []string `json:"waits,omitempty"` } -// MovesNothing is whether the delivery's plan moves no module and adds none: nothing for a walk to do. A -// delivery with no plan known is not said to move nothing. +// MovesNothing is whether the delivery's plan moves no module the mesh holds: nothing for a walk to do. A +// module it adds is not moved: the controller builds and registers it and sends it nowhere, and opens no walk +// for it (novox/hq issues 300 and 319). A delivery with no plan known is not said to move nothing. func (d *Delivery) MovesNothing() bool { - return d.Plan != nil && len(d.Plan.Moved)+len(d.Plan.New) == 0 + return d.Plan != nil && len(d.Plan.Moved) == 0 } // Step is one machine's part of a delivering walk. @@ -338,6 +343,8 @@ type WalkSeen struct { Tier int `json:"tier"` Tiers int `json:"tiers"` Updated time.Time `json:"updated"` + // Sent are the modules the walk sent to a machine, as its record says. + Sent []string `json:"sent,omitempty"` } // Seen is a walk as a delivery keeps it. @@ -350,6 +357,12 @@ func (w Walk) Seen() *WalkSeen { s.GoBy = d.By s.StoppedBy = d.Stopped } + for name, m := range w.Modules { + if m != nil && (m.SentAt != nil || m.FirstAt != nil) { + s.Sent = append(s.Sent, name) + } + } + sort.Strings(s.Sent) return s } diff --git a/modules/mesh-delivery/cmd/mesh-delivery/table.go b/modules/mesh-delivery/cmd/mesh-delivery/table.go index d6e1d7dd..df43b3e3 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/table.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/table.go @@ -34,17 +34,26 @@ const ( // replayed from the bus when it first held the seat. It reached the machines before this owner existed, // and nothing is delivered, released or put back for it (novox/hq issue 313). Retired State = "retired" + // Covered is a merge whose change reached the machines through a later one: every module it moves was + // sent by a finished walk of a commit merged after it into the same trunk, which therefore contains it. It + // is not walked itself, and it is no failure: its pull request says covered, as a success (novox/hq issue + // 319). + Covered State = "covered" ) // AllStates are every state a delivery can be in, for the table's own test and its verb. var AllStates = []State{Proposed, Checked, Ready, Rejected, Published, Delivering, Held, Delivered, Failed, - Superseded, Stopped, Retired} + Superseded, Stopped, Retired, Covered} // Final is whether nothing follows a state. func (s State) Final() bool { - return s == Delivered || s == Failed || s == Superseded || s == Stopped || s == Retired + return s == Delivered || s == Failed || s == Superseded || s == Stopped || s == Retired || s == Covered } +// Reached is whether a final state says the change reached the machines, or had nothing to reach them +// with: what a group counts as delivered. +func (s State) Reached() bool { return s == Delivered || s == Retired || s == Covered } + // Event is what moves a delivery. type Event string @@ -72,6 +81,8 @@ const ( EvStop Event = "stop" // a person stopped it, or its group did EvHistory Event = "history" // made from the forge's word for a merge long past: history EvRetire Event = "retire" // a person's word that a held delivery is history, never to be walked + EvCovered Event = "covered" // a later merge into the same trunk delivered every module it moves + EvLifted Event = "lifted" // what held it for a person is no longer so: its group's composed check passed ) // Facts are what a guard reads beyond the delivery itself: the moment, and who asks, for an act. @@ -91,6 +102,8 @@ type Row struct { Holds func(d *Delivery, f Facts) error // Act says the row is taken only when asked — a person's or H2's — never by settle. Act bool + // Says is the transition's why when nobody gave one: the fact that moved it, rather than the guard. + Says func(d *Delivery) string } func unless(cond bool, why string) error { @@ -161,6 +174,26 @@ var Table = []Row{ {From: []State{Checked}, Event: EvRefused, To: Rejected, Guard: "the gate failed or could not run, or the repository's own check failed", Holds: func(d *Delivery, f Facts) error { return unless(!d.Check.Passes(), "the verdict passes") }}, + {From: []State{Proposed, Checked, Rejected, Held}, Event: EvDone, To: Delivered, + Guard: "merged, and its plan builds nothing for a walk to move — nothing at all, or only a new module the " + + "controller builds and sends nowhere: nothing to deliver and nothing for a person to decide, whatever its check said", + Holds: func(d *Delivery, f Facts) error { + return unless(d.MergedAs != "" && d.MovesNothing() && d.Walk == nil, "not merged, or a walk moves it") + }, + Says: func(d *Delivery) string { return "merged, and nothing for a walk to move: " + d.Plan.Summary }}, + {From: []State{Ready, Held}, Event: EvCovered, To: Covered, + Guard: "merged, no walk of its own open, and every module it moves sent by a finished walk of a commit merged " + + "after it into the same trunk — which contains it, the trunk being protected: its change already runs", + Holds: func(d *Delivery, f Facts) error { + switch { + case d.MergedAs == "" || d.CoveredBy == "": + return errors.New("not merged, or nothing later carried its modules") + case d.Walk != nil && (d.Walk.State == walkBuilding || d.Walk.State == walkRolling): + return fmt.Errorf("its own walk %s is open: the controller delivers it", d.Walk.ID) + } + return nil + }, + Says: func(d *Delivery) string { return "covered: " + d.CoveredBy }}, {From: []State{Proposed, Checked, Rejected}, Event: EvMergedUnchecked, To: Held, Guard: "merged without a passing check — a verdict known with it is taken first: only a person decides that it goes on", Holds: func(d *Delivery, f Facts) error { return unless(d.MergedAs != "", "not merged") }}, @@ -208,6 +241,13 @@ var Table = []Row{ return unless(d.Walk != nil && d.Walk.Started() && (!d.Walk.Waited() || d.Walk.LetGoByAPerson()), "its walk was not started by a person or the controller's own path") }}, + {From: []State{Held}, Event: EvLifted, To: Published, + Guard: "held only for its group's composed check, which has since passed or warned for the heads that merged: " + + "it is published again and waits for its turn", + Holds: func(d *Delivery, f Facts) error { + return unless(d.Lifted != "" && d.HeldWhy == "" && d.MergedAs != "" && walkCanGoOn(d), "nothing lifted its hold") + }, + Says: func(d *Delivery) string { return "lifted: " + d.Lifted }}, {From: []State{Held}, Event: EvRelease, To: Delivering, Act: true, Guard: "a person's decision, with why", Holds: func(d *Delivery, f Facts) error { @@ -240,6 +280,15 @@ var Table = []Row{ Holds: func(d *Delivery, f Facts) error { return unless(f.Why != "" && f.By != "", "a stop says who and why") }}, } +// walkCanGoOn is whether a delivery published again would be delivered by what it has: its own walk still +// open, or nothing for a walk to move. +func walkCanGoOn(d *Delivery) bool { + if d.Walk == nil { + return d.MovesNothing() + } + return d.Walk.State == walkBuilding || d.Walk.State == walkRolling +} + // historyLate is how long after its merge a merge heard only from the forge's word is history: the forge // announces a merge within seconds, so one heard this late was replayed from the bus's history. const historyLate = 15 * time.Minute @@ -309,6 +358,9 @@ func Apply(d *Delivery, ev Event, act bool, f Facts) (Transition, error) { return Transition{}, ErrRefused{ID: d.ID, From: d.State, Event: ev, Why: "its guard does not hold: " + err.Error()} } why := f.Why + if why == "" && r.Says != nil { + why = r.Says(d) + } if why == "" { why = r.Guard } diff --git a/modules/mesh-delivery/cmd/mesh-delivery/table_test.go b/modules/mesh-delivery/cmd/mesh-delivery/table_test.go index 3ea121c3..73cbde8e 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/table_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/table_test.go @@ -55,11 +55,19 @@ func aDeliveryFor(r Row) (*Delivery, Facts) { case EvHold: d.HeldWhy, d.MergedAs = "its group's check did not pass", "aaaa1111" case EvDone: - if r.From[0] == Published { + switch { + case r.From[0] == Published: d.Plan = &DeliveryPlan{Summary: "moves nothing"} - } else { + case r.From[0] == Proposed: + d.MergedAs, d.Plan = "aaaa1111", &DeliveryPlan{Summary: "builds nothing"} + default: d.Walk = &WalkSeen{ID: "plan-1", State: walkDone} } + case EvCovered: + d.MergedAs, d.CoveredBy = "aaaa1111", "delivered with novox/app@bbbb (merged as bbbb2222, walk plan-2): app" + case EvLifted: + d.MergedAs, d.Lifted = "aaaa1111", "its group's composed check passed" + d.Walk = &WalkSeen{ID: "plan-1", State: walkBuilding, Waits: true, WaitedFor: "mesh-delivery"} case EvFailed: d.Walk = &WalkSeen{ID: "plan-1", State: walkFailed} case EvSuperseded: @@ -104,7 +112,7 @@ func TestEveryRowOfTheTableIsTakenWhenItsGuardHoldsAndRefusedWhenNot(t *testing. func TestEveryPairTheTableDoesNotHoldIsRefusedByName(t *testing.T) { events := []Event{EvAnnounced, EvAppeared, EvAdopted, EvChecked, EvAccepted, EvRefused, EvRecheck, EvReask, EvNewHead, EvClosed, EvMerged, EvMergedUnchecked, EvGo, EvHold, EvRelease, EvDone, EvFailed, EvSuperseded, EvStop, - EvHistory, EvRetire} + EvHistory, EvRetire, EvCovered, EvLifted} for _, from := range append([]State{None}, AllStates...) { for _, ev := range events { for _, act := range []bool{false, true} { @@ -140,10 +148,16 @@ func TestTheTableKeepsItsRules(t *testing.T) { t.Errorf("%s goes back to %s by %s", from, r.To, r.Event) } } - // The trunk rule: only a ready delivery is published, by its merge; nothing off the trunk delivers. - if r.To == Published && (r.Event != EvMerged || len(r.From) != 1 || r.From[0] != Ready) { + // The trunk rule: only a ready delivery is published, by its merge — or a held one published again when + // what held it is gone (issue 316); nothing off the trunk delivers. + if r.To == Published && !(r.Event == EvMerged && len(r.From) == 1 && r.From[0] == Ready) && + !(r.Event == EvLifted && len(r.From) == 1 && r.From[0] == Held) { t.Errorf("%v —%s→ published: only a ready delivery's merge publishes", r.From, r.Event) } + // Nothing a person did not decide ends a delivery as stopped or failed by observation from held. + if contains(r.From, Held) && !r.Act && (r.To == Stopped || r.To == Failed) { + t.Errorf("held —%s→ %s by observation", r.Event, r.To) + } if r.To == Delivering { for _, from := range r.From { if from == Proposed || from == Checked || from == Ready || from == Rejected { From 6327f297ea30cf72925151905c3c16e0ddfcb739 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 9 Oct 2026 02:57:18 +0200 Subject: [PATCH 2/3] Cover a held merge only when the forge says a later delivered merge contains it (review of #150) Merge order on a protected trunk was the whole proof; the review asked for ancestry. The gitea module gains gitea_contains, a read-only compare of two commits (total_commits 0 is contained), and mesh-delivery asks it once per pair, off the lock like its plan ask, keeping the answer on the delivery. A forge that cannot say leaves the merge held, not covered. Merge order stays the cheap first filter. A later delivery now carries a module only when its own check passed (so a merge held without a passing check is closed only by one that passed), its walk is of its own merge commit on the same trunk, and its steps say the module reached every machine running it, not one. The words say what is known: a later merge that contains it was delivered. --- modules/gitea/checks.ts | 17 +++ modules/gitea/client.ts | 6 + modules/gitea/test/checks.test.ts | 17 +++ modules/gitea/tools/index.ts | 20 +++- .../cmd/mesh-delivery/covered_test.go | 91 +++++++++++++-- .../cmd/mesh-delivery/effects.go | 2 +- .../cmd/mesh-delivery/fakes_test.go | 21 +++- .../mesh-delivery/cmd/mesh-delivery/holder.go | 106 ++++++++++++++---- .../cmd/mesh-delivery/manifest_test.go | 2 +- .../mesh-delivery/cmd/mesh-delivery/model.go | 25 +++-- .../mesh-delivery/cmd/mesh-delivery/ports.go | 20 ++++ .../mesh-delivery/cmd/mesh-delivery/table.go | 18 +-- modules/mesh-delivery/module.json | 3 +- 13 files changed, 292 insertions(+), 56 deletions(-) diff --git a/modules/gitea/checks.ts b/modules/gitea/checks.ts index 672ec03b..9208fcc7 100644 --- a/modules/gitea/checks.ts +++ b/modules/gitea/checks.ts @@ -81,3 +81,20 @@ export function mergeableUnder(branch: string, rule: BranchProtection | null | u blocking.map((b) => `${b.context} (${b.state})`).join(", "); return { branch, protected: !!rule, required, blocking, mergeable: blocking.length === 0, says }; } + +/** A commit, by its sha or the start of one: what `gitea_contains` takes, so a branch name or an expression + * never reaches the compare's path. */ +export function isCommit(ref: string): boolean { + return /^[0-9a-f]{7,40}$/i.test(ref); +} + +/** Whether a commit is contained in another — an ancestor of it, or it (novox/hq ADR 0270) — from the forge's + * compare of `in...commit`: the commits on `commit` that `in` lacks. None is contained. An answer that does not + * say how many is no answer. */ +export function containedFrom(compare: unknown): { contains: boolean; ahead: number } { + const total = (compare as Record | null)?.total_commits; + if (typeof total !== "number" || !Number.isInteger(total) || total < 0) { + throw new Error("the forge's compare said no total_commits"); + } + return { contains: total === 0, ahead: total }; +} diff --git a/modules/gitea/client.ts b/modules/gitea/client.ts index fe8e4234..10577d13 100644 --- a/modules/gitea/client.ts +++ b/modules/gitea/client.ts @@ -355,6 +355,12 @@ export class GiteaClient { return { sha: String(raw?.sha ?? ""), state: String(raw?.state ?? ""), statuses: Array.isArray(raw?.statuses) ? raw.statuses : [] }; } + /** The forge's compare of two commits, `base...head`: the commits on head that base lacks, and how many + * (novox/hq ADR 0270). Reads only. */ + async compare(owner: string, repo: string, base: string, head: string): Promise { + return this.request(`/repos/${owner}/${repo}/compare/${encodeURIComponent(base)}...${encodeURIComponent(head)}`); + } + /** Replace a comment's body: the delivery's view, kept current in place (novox/hq ADR 0239). */ async editComment(owner: string, repo: string, id: number, body: string): Promise<{ id: number; html_url: string }> { const c = await this.request(`/repos/${owner}/${repo}/issues/comments/${id}`, { method: "PATCH", body: JSON.stringify({ body }) }); diff --git a/modules/gitea/test/checks.test.ts b/modules/gitea/test/checks.test.ts index b29062bb..a6dee70a 100644 --- a/modules/gitea/test/checks.test.ts +++ b/modules/gitea/test/checks.test.ts @@ -46,3 +46,20 @@ test("no rule requires nothing; a rule that cannot be read says nothing, never y assert.equal(unread.mergeable, null); assert.match(unread.says, /could not be read/); }); + +// novox/hq ADR 0270: a held delivery is covered only when a later delivered merge contains it. +test("a commit is contained in another when the compare finds nothing on it the other lacks", async () => { + const { containedFrom, isCommit } = await import("../checks.ts"); + for (const [compare, contains, ahead] of [ + [{ total_commits: 0, commits: [] }, true, 0], + [{ total_commits: 3, commits: [{}, {}, {}] }, false, 3], + ] as const) { + assert.deepEqual(containedFrom(compare), { contains, ahead }); + } + for (const bad of [{}, { total_commits: "0" }, { total_commits: -1 }, null]) { + assert.throws(() => containedFrom(bad), /total_commits/); + } + assert.equal(isCommit("6aabe8e"), true); + assert.equal(isCommit("90516fc0a1b2c3d4e5f60718293a4b5c6d7e8f90"), true); + for (const bad of ["main", "6aabe8e...90516fc", "../x", "6aabe8", ""]) assert.equal(isCommit(bad), false, bad); +}); diff --git a/modules/gitea/tools/index.ts b/modules/gitea/tools/index.ts index f18281d8..33b70d41 100644 --- a/modules/gitea/tools/index.ts +++ b/modules/gitea/tools/index.ts @@ -12,7 +12,7 @@ import { registerModuleTools, type ToolDefinition } from "@novox/mesh-sdk/tools" import { emit } from "@novox/mesh-sdk/events"; import { GiteaClient, type BranchProtection } from "../client.js"; import { appendNote, deliveryStatus, run, viewBody, viewComment } from "../delivery.js"; -import { mergeableUnder, newestByContext } from "../checks.js"; +import { containedFrom, isCommit, mergeableUnder, newestByContext } from "../checks.js"; /** The forge's container, where its repositories and git are: the note is written there (novox/hq ADR 0239). */ const forgeContainer = process.env.MESH_GITEA_CONTAINER || "gitea"; @@ -492,6 +492,24 @@ export function getGiteaTools(gitea: GiteaClient): ToolDefinition[] { merge: mergeableUnder(base, rule, statuses, unreadable) }; }, }, + { + name: "gitea_contains", + description: "Whether a commit is contained in another — an ancestor of it, or the same — from the forge's compare of the two: what mesh-delivery reads before it says a held merge is covered by a later one (novox/hq ADR 0270). Commits only, by sha or the start of one. Reads only.", + input: { + owner: { type: "string", description: "the repository owner" }, + repo: { type: "string", description: "the repository name" }, + commit: { type: "string", description: "the commit asked about" }, + in: { type: "string", description: "the commit it may be contained in" }, + }, + run: async (args) => { + const owner = String(args.owner ?? "").trim(), repo = String(args.repo ?? "").trim(); + const commit = String(args.commit ?? "").trim(), within = String(args.in ?? "").trim(); + if (!owner || !repo) throw new Error("name the repository: owner and repo"); + if (!isCommit(commit) || !isCommit(within)) throw new Error("commit and in are commits: a sha, or the start of one"); + const { contains, ahead } = containedFrom(await gitea.compare(owner, repo, within, commit)); + return { commit, in: within, contains, ahead }; + }, + }, { name: "gitea_commit_status", description: "Set one of the mesh's statuses on a commit (mesh/delivery, mesh/delivery-group): pending, success, error, failure or warning, a short description, and the page it links to. The merge check's statuses (mesh/merge-gate, mesh/repo-check) take pending only — a recheck putting them back until the fresh verdict sets them.", diff --git a/modules/mesh-delivery/cmd/mesh-delivery/covered_test.go b/modules/mesh-delivery/cmd/mesh-delivery/covered_test.go index 62686249..8ec081e4 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/covered_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/covered_test.go @@ -133,19 +133,33 @@ func TestAMergeALaterDeliveredWalkCarriedIsCovered(t *testing.T) { modules []string // Whether this one was held for a person before the later merge was delivered. heldFirst bool - want State + // What the forge says of the later merge containing this one: yes, no, or away until asked again. + forge string + want State }{ {"carried every module, delivered after this one was held", "main", 35, walkDone, - []string{"build-agent", "mesh-controller"}, true, Covered}, + []string{"build-agent", "mesh-controller"}, true, "yes", Covered}, {"carried every module, delivered before this one was due to be held", "main", 35, walkDone, - []string{"build-agent", "mesh-controller", "route-proxy"}, false, Covered}, - {"carried one of its two modules", "main", 35, walkDone, []string{"mesh-controller"}, true, Held}, - {"merged before this one", "main", 25, walkDone, []string{"build-agent", "mesh-controller"}, true, Held}, - {"merged into another branch", "release", 35, walkDone, []string{"build-agent", "mesh-controller"}, true, Held}, - {"its walk failed", "main", 35, walkFailed, []string{"build-agent", "mesh-controller"}, true, Held}, + []string{"build-agent", "mesh-controller", "route-proxy"}, false, "yes", Covered}, + {"carried one of its two modules", "main", 35, walkDone, []string{"mesh-controller"}, true, "yes", Held}, + {"merged before this one", "main", 25, walkDone, []string{"build-agent", "mesh-controller"}, true, "yes", Held}, + {"merged into another branch", "release", 35, walkDone, []string{"build-agent", "mesh-controller"}, true, "yes", Held}, + {"its walk failed", "main", 35, walkFailed, []string{"build-agent", "mesh-controller"}, true, "yes", Held}, + {"the forge says the later merge does not contain it", "main", 35, walkDone, + []string{"build-agent", "mesh-controller"}, true, "no", Held}, + {"the forge could not say at first, then said it does", "main", 35, walkDone, + []string{"build-agent", "mesh-controller"}, true, "away", Covered}, } { t.Run(c.name, func(t *testing.T) { w := newWorld(t) + pair := merge + " in nnnnnnnnnnnn8888" + switch c.forge { + case "yes", "away": + w.forge.contains[pair] = true + case "no": + w.forge.contains[pair] = false + } + w.forge.containsDown = c.forge == "away" p := pr(ctl, 1, head, "feat/one", "cmd/a.go") w.h.PullUpdated(p) w.h.Checked(verdictMoving(ctl, 1, head, "mesh-controller", "build-agent")) @@ -172,13 +186,25 @@ func TestAMergeALaterDeliveredWalkCarriedIsCovered(t *testing.T) { t.Fatal(err) } w.settleAll() + if c.forge == "away" { + // A forge that cannot say is "not yet covered": held, and asked again a minute on. + if s := w.state(IDOf(ctl, head)); s != Held { + t.Fatalf("with the forge away it is %s", s) + } + w.forge.mu.Lock() + w.forge.containsDown = false + w.forge.mu.Unlock() + w.later(2 * time.Minute) + w.settleAll() + } d := w.delivery(IDOf(ctl, head)) if d.State != c.want { t.Fatalf("it is %s (%s), want %s", d.State, d.HeldWhy, c.want) } s := statusOn(w, ctl, head) if c.want == Covered { - if !strings.HasPrefix(s, "success covered") || !strings.Contains(s, "nnnnnnnn") { + if !strings.HasPrefix(s, "success covered: a later merge that contains it was delivered") || + !strings.Contains(s, "nnnnnnnn") { t.Fatalf("its pull request's mesh/delivery says %q", s) } last := d.Transitions[len(d.Transitions)-1] @@ -189,6 +215,22 @@ func TestAMergeALaterDeliveredWalkCarriedIsCovered(t *testing.T) { if strings.HasPrefix(s, "error") { t.Fatalf("its pull request's mesh/delivery is an error: %q", s) } + // Asked once for the pair, however many ticks read it — once more only after the forge was away. + asked := 0 + for _, a := range w.forge.containsAsks { + if a == pair { + asked++ + } + } + candidate := c.base == "main" && c.at > 30 && c.state == walkDone && len(c.modules) >= 2 + switch { + case !candidate && asked != 0: + t.Fatalf("the forge was asked whether %s for a merge order and walk that cannot cover it", pair) + case candidate && c.forge != "away" && asked != 1: + t.Fatalf("the forge was asked %d time(s) whether %s, want once", asked, pair) + case candidate && c.forge == "away" && asked < 2: + t.Fatalf("the forge, away, was asked %d time(s) whether %s: never again", asked, pair) + } }) } } @@ -212,6 +254,7 @@ func TestACoveredGroupMemberLetsTheNextOneGo(t *testing.T) { w.h.PullMerged(mergedAt(pl, "m2m2m2m2m2m2", t0.Add(31*time.Minute))) w.h.WalkMoved(w.ctl.put(aWalk("plan-lab", lab, "m2m2m2m2m2m2", true))) w.settleAll() + w.forge.contains["m1m1m1m1m1m1 in m3m3m3m3m3m3"] = true deliverLater(w, ctl, 3, "main", "e3e3e3e3e3e3e3e3", "m3m3m3m3m3m3", t0.Add(35*time.Minute), walkDone, "mesh-controller") w.later(20 * time.Minute) if err := w.h.Reconcile(); err != nil { @@ -241,7 +284,7 @@ func TestHeldDeliveriesReadBackCloseByThemselvesOnce(t *testing.T) { coverer := func(repo, module string) *Delivery { return &Delivery{ID: IDOf(repo, "c0c0c0c0c0c0c0c0"), Repository: repo, Commit: "c0c0c0c0c0c0c0c0", Number: 90, Base: "main", State: Delivered, Since: merged2.Add(time.Hour), Created: merged2.Add(-time.Hour), - MergedAs: "f0f0f0f0f0f0f0f0", MergedAt: merged2, Plan: planMoving(repo, module), + MergedAs: "f0f0f0f0f0f0f0f0", MergedAt: merged2, Plan: planMoving(repo, module), Check: &Verdict{Gate: "pass"}, Walk: &WalkSeen{ID: "plan-90", Commit: "f0f0f0f0f0f0f0f0", State: walkDone}, Steps: []Step{{Machine: "anchor", Module: module, State: StepPassed}, {Machine: "the rest running it", Module: module, State: StepSent}}} } @@ -252,6 +295,12 @@ func TestHeldDeliveriesReadBackCloseByThemselvesOnce(t *testing.T) { } const unwalkedWhy = "merged, and the controller opened no walk for it within 10m0s — nothing it holds follows that branch, " + "or the merge was not heard: a person decides" + withCoverer := func(d *Delivery, change func(c *Delivery)) []*Delivery { + c := coverer(d.Repository, d.Plan.Moved[0]) + change(c) + return []*Delivery{d, c} + } + const uncheckedWhy = "merged without a passing check: only a person decides that it goes on" for _, c := range []struct { name string kept []*Delivery @@ -266,6 +315,24 @@ func TestHeldDeliveriesReadBackCloseByThemselvesOnce(t *testing.T) { nil, Delivered}, {"no walk opened, a later delivered walk carried its module", []*Delivery{held(ctl, planMoving(ctl, "mesh-controller"), unwalkedWhy), coverer(ctl, "mesh-controller")}, nil, Covered}, + {"merged without a passing check, a later delivered walk whose own check passed carried its module", + withCoverer(held(ctl, planMoving(ctl, "mesh-controller"), uncheckedWhy), func(*Delivery) {}), nil, Covered}, + {"merged without a passing check, the later delivery's own check did not pass", + withCoverer(held(ctl, planMoving(ctl, "mesh-controller"), uncheckedWhy), func(c *Delivery) { + c.Check = &Verdict{Gate: "fail"} + }), nil, Held}, + {"no walk opened, the later delivery's walk was of another commit", + withCoverer(held(ctl, planMoving(ctl, "mesh-controller"), unwalkedWhy), func(c *Delivery) { + c.Walk.Commit = "e0e0e0e0e0e0e0e0" + }), nil, Held}, + {"no walk opened, the later walk sent its module to its first machine only", + withCoverer(held(ctl, planMoving(ctl, "mesh-controller"), unwalkedWhy), func(c *Delivery) { + c.Steps = c.Steps[:1] + }), nil, Held}, + {"no walk opened, the later walk's branch is another", + withCoverer(held(ctl, planMoving(ctl, "mesh-controller"), unwalkedWhy), func(c *Delivery) { + c.Walk.Branch = "release" + }), nil, Held}, {"no walk opened, a later delivered walk carried another module", []*Delivery{held(ctl, planMoving(ctl, "mesh-controller"), unwalkedWhy), coverer(ctl, "route-proxy")}, nil, Held}, {"held for its group, its own walk superseded, a later delivered walk carried its module", @@ -298,7 +365,9 @@ func TestHeldDeliveriesReadBackCloseByThemselvesOnce(t *testing.T) { } } now := merged2.Add(30 * time.Hour) - w := newWorldOver(t, store, newFakeController(), newFakeForge(), now) + forge := newFakeForge() + forge.contains[merge+" in f0f0f0f0f0f0f0f0"] = true + w := newWorldOver(t, store, newFakeController(), forge, now) if err := w.h.Reconcile(); err != nil { t.Fatal(err) } @@ -312,7 +381,7 @@ func TestHeldDeliveriesReadBackCloseByThemselvesOnce(t *testing.T) { t.Fatalf("its pull request's mesh/delivery says %q", s) } // A restart over what it kept: nothing moves again. - again := newWorldOver(t, store, newFakeController(), newFakeForge(), now.Add(time.Hour)) + again := newWorldOver(t, store, newFakeController(), forge, now.Add(time.Hour)) if err := again.h.Reconcile(); err != nil { t.Fatal(err) } diff --git a/modules/mesh-delivery/cmd/mesh-delivery/effects.go b/modules/mesh-delivery/cmd/mesh-delivery/effects.go index 83fb8c84..84780839 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/effects.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/effects.go @@ -108,7 +108,7 @@ func statusOf(d *Delivery) (string, string) { case Retired: return "success", "history: merged before this owner delivered anything, never walked" case Covered: - return "success", clip("covered: "+d.CoveredBy, 140) + return "success", clip("covered: a later merge that contains it was delivered: "+d.CoveredBy, 140) } return "pending", string(d.State) } diff --git a/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go b/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go index 1ecacccf..26a02a39 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go @@ -235,11 +235,15 @@ type fakeForge struct { required []string protectionDown bool asked []string + // contains answers gitea_contains by " in "; a pair it does not hold is a forge that cannot say. + contains map[string]bool + containsDown bool + containsAsks []string } func newFakeForge() *fakeForge { return &fakeForge{notes: map[string][]string{}, views: map[string]string{}, statuses: map[string]string{}, - pulls: map[string]ForgePull{}} + pulls: map[string]ForgePull{}, contains: map[string]bool{}} } func (f *fakeForge) Note(owner, repo, commit, line string) error { @@ -347,6 +351,21 @@ func (f *fakeForge) Statuses(owner, repo string, number int, commit, base string return out, nil } +// Contains answers what the fake was told of two commits. +func (f *fakeForge) Contains(owner, repo, commit, in string) (bool, error) { + f.mu.Lock() + defer f.mu.Unlock() + f.containsAsks = append(f.containsAsks, commit+" in "+in) + if f.containsDown || f.down { + return false, errors.New("the forge is away") + } + yes, said := f.contains[commit+" in "+in] + if !said { + return false, fmt.Errorf("the forge has no commit %s or %s", commit, in) + } + return yes, nil +} + func (f *fakeForge) notesOn(commit string) []string { f.mu.Lock() defer f.mu.Unlock() diff --git a/modules/mesh-delivery/cmd/mesh-delivery/holder.go b/modules/mesh-delivery/cmd/mesh-delivery/holder.go index a5114dce..060c3c2a 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/holder.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/holder.go @@ -764,7 +764,13 @@ func (h *Holder) plan() []func() { continue } if d.CoveredBy == "" { - d.CoveredBy = h.coverOf(d) + var ask []string + d.CoveredBy, ask = h.coverOf(d) + for _, in := range ask { + if a := h.askContains(d, in, now); a != nil { + asks = append(asks, a) + } + } } if ts := settle(d, Facts{Now: now}); len(ts) > 0 { h.moved(d, ts) @@ -819,26 +825,42 @@ func (h *Holder) unwalkedPast(d *Delivery, now time.Time) bool { now.Sub(d.MergedAt) > unwalked && h.reconciled.After(d.MergedAt.Add(unwalked)) } -// coverOf says which later merges cover a merged delivery, or nothing (novox/hq issue 319): for every module -// its plan moves, a delivered delivery of the same repository and trunk, merged after it — so its merge commit -// contains this one's, the trunk being protected and only ever merged into — whose finished walk sent that -// module. Read from this owner's own records: the forge's merge times and the walks' records. -func (h *Holder) coverOf(d *Delivery) string { +// coverOf says which later merges cover a merged delivery, or nothing, and which later merge commits the forge +// must still be asked about (novox/hq ADR 0270). A module is covered by a later delivery that carries it (see +// carries) and whose merge commit the forge said contains this one's merge. Merge order is the cheap filter; +// ancestry decides. The forge is asked only once every module has a candidate, once per pair, and its answer +// is kept on the delivery: an answer not known yet is "not yet covered". +func (h *Holder) coverOf(d *Delivery) (string, []string) { if d.MergedAs == "" || d.MergedAt.IsZero() || d.Base == "" || d.Plan == nil || len(d.Plan.Moved) == 0 { - return "" + return "", nil + } + candidates := map[string][]*Delivery{} + for _, module := range d.Plan.Moved { + for _, c := range h.deliveries { + if carries(c, d, module) { + candidates[module] = append(candidates[module], c) + } + } + if len(candidates[module]) == 0 { + return "", nil // nothing later carried it: nothing to ask + } } carried := map[string][]string{} var by []*Delivery + var ask []string for _, module := range d.Plan.Moved { var first *Delivery - for _, c := range h.deliveries { - if carries(c, d, module) && (first == nil || c.MergedAt.Before(first.MergedAt) || - c.MergedAt.Equal(first.MergedAt) && c.ID < first.ID) { + for _, c := range candidates[module] { + yes, known := d.ContainedIn[c.MergedAs] + if !known && !containsString(ask, c.MergedAs) { + ask = append(ask, c.MergedAs) + } + if yes && (first == nil || c.MergedAt.Before(first.MergedAt) || c.MergedAt.Equal(first.MergedAt) && c.ID < first.ID) { first = c } } if first == nil { - return "" + return "", ask } if carried[first.ID] == nil { by = append(by, first) @@ -850,29 +872,67 @@ func (h *Holder) coverOf(d *Delivery) string { for _, c := range by { mods := carried[c.ID] sort.Strings(mods) - parts = append(parts, fmt.Sprintf("delivered with %s (merged as %s, walk %s): %s", c.ID, shortOf(c.MergedAs, 8), + parts = append(parts, fmt.Sprintf("%s (merged as %s into %s, walk %s): %s", c.ID, shortOf(c.MergedAs, 8), d.Base, c.Walk.ID, strings.Join(mods, ", "))) } - return fmt.Sprintf("%s — merged into %s after %s, which it therefore contains", strings.Join(parts, "; "), d.Base, - shortOf(d.MergedAs, 8)) + return strings.Join(parts, "; "), nil } -// carries is whether c delivered a module after d, from a commit that contains d's merge. +// carries is whether c delivered a module after d: of the same repository, merged later into the same trunk, +// delivered on its own passing check — a merge held without one is covered only by a later merge that passed +// its own — by its own walk, of its merge commit on that trunk, which sent the module to every machine running +// it. Whether c's merge commit contains d's is the forge's to say (coverOf). func carries(c, d *Delivery, module string) bool { - if c.ID == d.ID || c.State != Delivered || !strings.EqualFold(c.Repository, d.Repository) || + if c.ID == d.ID || c.State != Delivered || !c.Check.Passes() || !strings.EqualFold(c.Repository, d.Repository) || !strings.EqualFold(c.Base, d.Base) || c.MergedAs == "" || sameCommit(c.MergedAs, d.MergedAs) || - !c.MergedAt.After(d.MergedAt) || c.Walk == nil || c.Walk.State != walkDone { + !c.MergedAt.After(d.MergedAt) || c.Walk == nil || c.Walk.State != walkDone || !sameCommit(c.Walk.Commit, c.MergedAs) || + c.Walk.Branch != "" && !strings.EqualFold(c.Walk.Branch, d.Base) { return false } - if containsString(c.Walk.Sent, module) { - return true - } + everywhere := false for _, s := range c.Steps { - if s.Module == module && (s.State == StepPassed || s.State == StepSent) { - return true + if s.Module != module { + continue + } + switch { + case s.State == StepFailed || s.State == StepRolledBack: + return false + case s.State == StepSent && (s.Machine == stepEveryMachine || s.Machine == stepTheRest): + everywhere = true } } - return false + return everywhere +} + +// askContains is the ask of the forge whether a later merge commit contains a delivery's merge: at most once a +// minute while the forge cannot say, and never again once it said — its answer is kept on the delivery. +func (h *Holder) askContains(d *Delivery, in string, now time.Time) func() { + key := d.ID + " in " + in + if tried, was := h.askTried[key]; was && now.Sub(tried) < time.Minute { + return nil + } + h.askTried[key] = now + id, owner, repo, merged := d.ID, d.Owner(), d.Repo(), d.MergedAs + return func() { + yes, err := h.Forge.Contains(owner, repo, merged, in) + h.mu.Lock() + defer h.mu.Unlock() + if err != nil { + h.Logf("[mesh-delivery] %s: whether %s contains its merge cannot be read yet (%v): not yet covered", id, + shortOf(in, 12), err) + return + } + delete(h.askTried, key) + d := h.deliveries[id] + if d == nil { + return + } + if d.ContainedIn == nil { + d.ContainedIn = map[string]bool{} + } + d.ContainedIn[in] = yes + h.keep(d) + } } // heldForGroup is whether a member is held only because its group's composed check had not passed. diff --git a/modules/mesh-delivery/cmd/mesh-delivery/manifest_test.go b/modules/mesh-delivery/cmd/mesh-delivery/manifest_test.go index 05989ba2..43a2c526 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/manifest_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/manifest_test.go @@ -65,7 +65,7 @@ func TestItClaimsTheDeliverySeatAndFollowsWhatItHandles(t *testing.T) { want = append(want, "seat:"+ControllerSeat+"."+v) } want = append(want, "gitea.gitea_note_append", "gitea.gitea_delivery_view", "gitea.gitea_commit_status", - "gitea.gitea_commit_statuses") + "gitea.gitea_commit_statuses", "gitea.gitea_contains") if !reflect.DeepEqual(m.Invokes, want) { t.Fatalf("invokes %v, wanted %v", m.Invokes, want) } diff --git a/modules/mesh-delivery/cmd/mesh-delivery/model.go b/modules/mesh-delivery/cmd/mesh-delivery/model.go index fa98d32f..ef31ee77 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/model.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/model.go @@ -65,6 +65,9 @@ type Delivery struct { // (novox/hq issue 319); Lifted why what held it for a person no longer holds (issue 316). CoveredBy string `json:"covered_by,omitempty"` Lifted string `json:"lifted,omitempty"` + // ContainedIn is the forge's answer, by a later merge commit, to whether that commit contains this one's + // merge (gitea_contains): asked once per pair, and kept (ADR 0270). + ContainedIn map[string]bool `json:"contained_in,omitempty"` // Walk is the controller's walk of its trunk commit, as last read; Steps the machines it reached. Walk *WalkSeen `json:"walk,omitempty"` Steps []Step `json:"steps,omitempty"` @@ -343,8 +346,8 @@ type WalkSeen struct { Tier int `json:"tier"` Tiers int `json:"tiers"` Updated time.Time `json:"updated"` - // Sent are the modules the walk sent to a machine, as its record says. - Sent []string `json:"sent,omitempty"` + // Branch is the branch the walk's commit was merged into; empty in a record kept before it was read. + Branch string `json:"branch,omitempty"` } // Seen is a walk as a delivery keeps it. @@ -357,12 +360,7 @@ func (w Walk) Seen() *WalkSeen { s.GoBy = d.By s.StoppedBy = d.Stopped } - for name, m := range w.Modules { - if m != nil && (m.SentAt != nil || m.FirstAt != nil) { - s.Sent = append(s.Sent, name) - } - } - sort.Strings(s.Sent) + s.Branch = w.Branch return s } @@ -379,6 +377,13 @@ func (s *WalkSeen) LetGoByAPerson() bool { return s != nil && strings.HasPrefix( // Open is whether the controller still works it. func (w Walk) Open() bool { return w.State == walkBuilding || w.State == walkRolling } +// The steps that say a module reached every machine running it: sent to all at once, or to the rest after its +// first machines passed. +const ( + stepEveryMachine = "every machine running it" + stepTheRest = "the rest running it" +) + // StepsOf are the machine steps a walk's record says, for one delivery: every module it moved, on its first // machines from its gate, and the rest once sent. func StepsOf(w Walk) []Step { @@ -392,7 +397,7 @@ func StepsOf(w Walk) []Step { m := w.Modules[name] if m == nil || m.FirstAt == nil { if m != nil && m.SentAt != nil && m.FirstAt == nil && m.Why == "" { - out = append(out, Step{Machine: "every machine running it", Module: name, Build: m.Build, + out = append(out, Step{Machine: stepEveryMachine, Module: name, Build: m.Build, State: StepSent, Since: *m.SentAt}) } continue @@ -419,7 +424,7 @@ func StepsOf(w Walk) []Step { out = append(out, Step{Machine: machine, Module: name, Build: m.Build, State: state, Since: since, Why: why}) } if m.SentAt != nil && state == StepPassed { - out = append(out, Step{Machine: "the rest running it", Module: name, Build: m.Build, State: StepSent, + out = append(out, Step{Machine: stepTheRest, Module: name, Build: m.Build, State: StepSent, Since: *m.SentAt}) } } diff --git a/modules/mesh-delivery/cmd/mesh-delivery/ports.go b/modules/mesh-delivery/cmd/mesh-delivery/ports.go index 8b775503..be2bb752 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/ports.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/ports.go @@ -76,6 +76,8 @@ type Forge interface { // Statuses reads a commit's statuses — a pull request's head's, given its number — and whether the branch // it merges into would let it merge. Reads only. Statuses(owner, repo string, number int, commit, base string) (*ForgeChecks, error) + // Contains is whether a commit is contained in another — an ancestor of it, or it. Reads only. + Contains(owner, repo, commit, in string) (bool, error) } // ForgeChecks is what the forge says of a commit's checks (the gitea module's gitea_commit_statuses). @@ -337,6 +339,24 @@ func (f toolForge) Statuses(owner, repo string, number int, commit, base string) return &c, nil } +func (f toolForge) Contains(owner, repo, commit, in string) (bool, error) { + raw, err := f.ask("gitea.gitea_contains", map[string]any{"owner": owner, "repo": repo, "commit": commit, "in": in}) + if err != nil { + return false, err + } + answer, output, ok := answerOf(raw) + if !ok { + return false, fmt.Errorf("the forge refused gitea_contains: %s", lastLine(output)) + } + var a struct { + Contains *bool `json:"contains"` + } + if err := json.Unmarshal(answer, &a); err != nil || a.Contains == nil { + return false, fmt.Errorf("the forge's gitea_contains is not readable: %s", clip(string(answer), 200)) + } + return *a.Contains, nil +} + // kvStore is the module's own state: `deliveries` and `groups`, one key each. type kvStore struct{} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/table.go b/modules/mesh-delivery/cmd/mesh-delivery/table.go index df43b3e3..e554cabf 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/table.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/table.go @@ -34,10 +34,11 @@ const ( // replayed from the bus when it first held the seat. It reached the machines before this owner existed, // and nothing is delivered, released or put back for it (novox/hq issue 313). Retired State = "retired" - // Covered is a merge whose change reached the machines through a later one: every module it moves was - // sent by a finished walk of a commit merged after it into the same trunk, which therefore contains it. It - // is not walked itself, and it is no failure: its pull request says covered, as a success (novox/hq issue - // 319). + // Covered is a merge a later one that contains it delivered: every module it moves was sent to every machine + // running it by the finished walk of a later merge into the same trunk, whose own check passed and which the + // forge says contains this one. It is not walked itself, and it is no failure: its pull request says covered, + // as a success (novox/hq ADR 0270). A revert merged in between is still possible: covered says what was + // delivered, not what runs. Covered State = "covered" ) @@ -182,8 +183,9 @@ var Table = []Row{ }, Says: func(d *Delivery) string { return "merged, and nothing for a walk to move: " + d.Plan.Summary }}, {From: []State{Ready, Held}, Event: EvCovered, To: Covered, - Guard: "merged, no walk of its own open, and every module it moves sent by a finished walk of a commit merged " + - "after it into the same trunk — which contains it, the trunk being protected: its change already runs", + Guard: "merged, no walk of its own open, and every module it moves sent to every machine running it by the " + + "finished walk of a later merge into the same trunk, whose own check passed and which the forge says contains " + + "it: a later merge that contains it was delivered", Holds: func(d *Delivery, f Facts) error { switch { case d.MergedAs == "" || d.CoveredBy == "": @@ -193,7 +195,9 @@ var Table = []Row{ } return nil }, - Says: func(d *Delivery) string { return "covered: " + d.CoveredBy }}, + Says: func(d *Delivery) string { + return "covered: a later merge that contains it was delivered: " + d.CoveredBy + }}, {From: []State{Proposed, Checked, Rejected}, Event: EvMergedUnchecked, To: Held, Guard: "merged without a passing check — a verdict known with it is taken first: only a person decides that it goes on", Holds: func(d *Delivery, f Facts) error { return unless(d.MergedAs != "", "not merged") }}, diff --git a/modules/mesh-delivery/module.json b/modules/mesh-delivery/module.json index 570e766f..a165b473 100644 --- a/modules/mesh-delivery/module.json +++ b/modules/mesh-delivery/module.json @@ -43,7 +43,8 @@ "gitea.gitea_note_append", "gitea.gitea_delivery_view", "gitea.gitea_commit_status", - "gitea.gitea_commit_statuses" + "gitea.gitea_commit_statuses", + "gitea.gitea_contains" ], "state": [ "deliveries", From 0bd50fce0ad6bbf47eca00f5d3a74d04753f445a Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 9 Oct 2026 03:23:19 +0200 Subject: [PATCH 3/3] The forge module's tool list test names the statuses and contains tools It had not been run before a merge: its expected list lacked gitea_commit_statuses since that tool came, and now gitea_contains. The catalogue's TypeScript check, merged in from #151, runs it. --- modules/gitea/test/token.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/gitea/test/token.test.ts b/modules/gitea/test/token.test.ts index b69edd9b..c6f3b4a7 100644 --- a/modules/gitea/test/token.test.ts +++ b/modules/gitea/test/token.test.ts @@ -411,7 +411,7 @@ test("the tools register once there is a way to a token, and the first call mint "gitea_list_branches", "gitea_delete_branch", "gitea_branch_protection_get", "gitea_branch_protection_set", - "gitea_note_append", "gitea_delivery_view", "gitea_commit_status", + "gitea_note_append", "gitea_delivery_view", "gitea_commit_statuses", "gitea_contains", "gitea_commit_status", "gitea_list_labels", "gitea_create_label", "gitea_api", ],