From 9e41438f1b647fe97920bbd304242150f7197217 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 21:15:31 +0100 Subject: [PATCH] A merge of a module running nowhere countable reads not yet on the machines in the gate and the tracker; a settle ask is taken away once answered or lapsed; pin mesh-sdk e44cbaf (review of issue 1280) --- .../cmd/mesh-delivery-gate/approval.go | 1 + .../cmd/mesh-delivery-gate/reached.go | 14 +++++++ .../cmd/mesh-delivery-gate/reached_test.go | 8 ++++ .../cmd/mesh-delivery-gate/settle.go | 37 +++++++++++++++++++ .../cmd/mesh-delivery-gate/settle_test.go | 26 +++++++++++++ modules/mesh-delivery-gate/go.mod | 2 +- modules/mesh-delivery-gate/go.sum | 4 +- .../cmd/mesh-issues/delivered_test.go | 16 ++++++-- .../mesh-issues/cmd/mesh-issues/delivery.go | 16 +++++++- 9 files changed, 116 insertions(+), 8 deletions(-) diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval.go index d835ffb4..9e0f7e96 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval.go @@ -299,6 +299,7 @@ func (h *Holder) headsHold(d *Delivery, c *ReadyRun) bool { // §3), and says so once on each pull request it covered (ADR 0421 §6). An ask for a head a newer one replaced ends too. // A Decline stands, and is not ended. Under the lock; the cancels run with the look's asks. func (h *Holder) endStaleAsks() { + h.endStaleSettles() for _, d := range h.deliveries { r := d.Ready if r == nil || r.Merged != nil || d.MergedAs != "" || r.Declined != nil || (r.AskID == "" && r.Approved == nil) || len(r.Heads) == 0 { diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/reached.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/reached.go index 5bf5fb37..6456be41 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/reached.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/reached.go @@ -54,6 +54,20 @@ func (h *Holder) machines(d *Delivery) string { switch { case !ok: return notKnownWhere + "the delivery view says no reach of " + d.Repository + } + // A merge that changed a module a node wants and runs nothing countable of is not yet there, whatever the reached head + // (the module holds no head back, ADR 0556 clause 2). + for _, mr := range r.Modules { + if mr.Nodes == 0 || mr.Floor != "" { + continue + } + for _, c := range mr.Changes { + if sameCommit(d.MergedAs, c.Commit) { + return notYetOn + mr.Waiting + } + } + } + switch { case r.Reached == "": return notYetOn + holdsSaid(r) case sameCommit(d.MergedAs, r.Reached): diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/reached_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/reached_test.go index 89493406..657a42eb 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/reached_test.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/reached_test.go @@ -49,6 +49,14 @@ func TestAMergedDeliverySaysWhetherItIsOnTheMachines(t *testing.T) { if got := machines(first); got != notYetOn+"web on ace runs nothing yet" { t.Errorf("no reached head: %q", got) } + // A module a node wants and runs nothing countable of holds no head back, and a merge that changed it is not yet there. + reach(sdk.Reach{Repository: "novox/web", Reached: third.MergedAs, ReachedSrc: 9, Modules: map[string]sdk.ModuleReach{ + "web": {FP: "src2:x", Nodes: 1, Waiting: "web on g14 runs nothing yet", Changes: []sdk.Change{{Commit: second.MergedAs, FP: "src2:x"}}}}}) + for d, want := range map[*Delivery]string{first: onMachines, second: notYetOn + "web on g14 runs nothing yet", third: onMachines} { + if got := machines(d); got != want { + t.Errorf("with a module running nowhere countable, %s: %q, want %q", d.ID, got, want) + } + } // The verbs say it; a delivery not merged says nothing of the machines. reach(sdk.Reach{Repository: "novox/web", Reached: third.MergedAs, ReachedSrc: 9}) lines := w.h.Deliveries("", "novox/web", "", true) diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/settle.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/settle.go index 85263657..a95ef111 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/settle.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/settle.go @@ -184,6 +184,7 @@ func (h *Holder) settled(key string, ans sdk.Answer) { } } if len(covered) == 0 { + h.Logf("[mesh-delivery] the answer to the settle ask %s covers no delivery that waits for it: nothing settled", key) return } sort.Slice(covered, func(i, j int) bool { return covered[i].ID < covered[j].ID }) @@ -212,4 +213,40 @@ func (h *Holder) settled(key string, ans sdk.Answer) { d.HeldWhy = "" h.moved(d, []Transition{t}) } + // Answered: the ask is taken away, as a merge ask is once acted on. + h.cancels = append(h.cancels, h.cancelSettle(key)) +} + +// cancelSettle takes a settle ask away from the gate's asks. +func (h *Holder) cancelSettle(key string) func() { + return func() { + if h.Operator == nil { + return + } + if err := h.Operator.Cancel(key); err != nil { + h.Logf("[mesh-delivery] the settle ask %s could not be taken away (taken away again at the next look): %v", key, err) + } + } +} + +// endStaleSettles takes away every settle ask nobody answered within its bound; its deliveries stay held and settle may ask +// again. Under the lock; the cancels run with the look's asks. +func (h *Holder) endStaleSettles() { + done := map[string]bool{} + for _, d := range h.deliveries { + s := d.Settle + if s == nil || s.Answered != nil || done[s.Key] || h.Now().Before(s.At.Add(AskLasts)) { + continue + } + done[s.Key] = true + h.Logf("[mesh-delivery] the settle ask %s lapsed with no answer: taken away, %s stays held", s.Key, d.ID) + h.cancels = append(h.cancels, h.cancelSettle(s.Key)) + } + for _, d := range h.deliveries { + if s := d.Settle; s != nil && done[s.Key] { + now := h.Now() + s.Answered, s.Answer = &now, "lapsed" + h.keep(d) + } + } } diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/settle_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/settle_test.go index 9ccb8cea..bbe88fae 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/settle_test.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/settle_test.go @@ -76,12 +76,38 @@ func TestSettleAsksTheOperatorOnceAndMovesOnlyOnTheAnswer(t *testing.T) { if again := w.op.ask(key); again.ID == ask.ID || a.Settle.Asking != 2 { t.Fatalf("asked again under %s (asking %d), want a new id", again.ID, a.Settle.Asking) } + // An answer under a settle key no delivery waits for moves nothing. + other, _ := sdk.MergeAskKey(sdk.SettleGroup([]string{"novox/web@ffffffffffff"}), []sdk.Head{{Repository: "novox/web", Number: 1, Head: "ffffffffffff"}}) + w.h.mu.Lock() + w.h.settled(other, sdk.Answer{Option: OptionSettle, Warrant: "x@now", Words: "chose Settle"}) + w.h.mu.Unlock() + if a.State != Held || b.State != Held || !strings.Contains(w.logbuf.String(), "covers no delivery") { + t.Fatalf("an answer for other ids moved %s, %s", a.State, b.State) + } w.op.answer(key, OptionSettle) w.h.hearAsks() + w.h.mu.Lock() + cancels := len(w.h.cancels) + w.h.mu.Unlock() + if cancels == 0 { + t.Error("the answered settle ask is not taken away") + } for _, d := range []*Delivery{a, b} { last := d.Transitions[len(d.Transitions)-1] if d.State != Merged || last.Event != EvSettled || last.By != "the operator" || !strings.Contains(last.Why, "chose Settle") { t.Errorf("%s after Settle: %s, %+v", d.ID, d.State, last) } } + // A settle ask nobody answers within its bound is taken away; its delivery stays held. + c := put("novox/web@999999999999", Held, HeldUnchecked) + if _, err := w.h.Settle([]string{c.ID}, "once more", aPerson); err != nil { + t.Fatal(err) + } + w.later(AskLasts + time.Minute) + w.h.mu.Lock() + w.h.endStaleSettles() + w.h.mu.Unlock() + if c.State != Held || c.Settle.Answer != "lapsed" { + t.Errorf("a lapsed settle ask: %s %+v", c.State, c.Settle) + } } diff --git a/modules/mesh-delivery-gate/go.mod b/modules/mesh-delivery-gate/go.mod index d219ce5f..1a02ef85 100644 --- a/modules/mesh-delivery-gate/go.mod +++ b/modules/mesh-delivery-gate/go.mod @@ -3,7 +3,7 @@ module mesh-delivery-gate go 1.26.0 require ( - git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026195749-33144781d652 + git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026201248-e44cbaf57fc3 github.com/nats-io/nats-server/v2 v2.11.17 github.com/nats-io/nats.go v1.54.0 ) diff --git a/modules/mesh-delivery-gate/go.sum b/modules/mesh-delivery-gate/go.sum index a8894095..73a8b6b4 100644 --- a/modules/mesh-delivery-gate/go.sum +++ b/modules/mesh-delivery-gate/go.sum @@ -1,5 +1,5 @@ -git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026195749-33144781d652 h1:R+gkczO84KN1nbTG3ubIfAAkbx/jTb+txW1fQVhtmSI= -git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026195749-33144781d652/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026201248-e44cbaf57fc3 h1:kOAR8GaTpOP9j2XNm3grqEfSwSDHnWbHKd+Qftkr/QE= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026201248-e44cbaf57fc3/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= github.com/antithesishq/antithesis-sdk-go v0.7.0-default-no-op h1:Z/MZK75wC/NSrkgqeNIa7jexam9uWzhLmFTSCPI/kn0= github.com/antithesishq/antithesis-sdk-go v0.7.0-default-no-op/go.mod h1:FQyySiasQQM8735Ddel3MRojmy4dA1IqCeyJ5jmPMbI= github.com/google/go-tpm v0.9.8 h1:slArAR9Ft+1ybZu0lBwpSmpwhRXaa85hWtMinMyRAWo= diff --git a/modules/mesh-issues/cmd/mesh-issues/delivered_test.go b/modules/mesh-issues/cmd/mesh-issues/delivered_test.go index 22a05f06..67a810be 100644 --- a/modules/mesh-issues/cmd/mesh-issues/delivered_test.go +++ b/modules/mesh-issues/cmd/mesh-issues/delivered_test.go @@ -139,11 +139,19 @@ func TestANewerRevisionCoversTheOlderMergesOfItsRepositoryAsTheForgeSaysAncestry h3.Contains = func(_, _, commit, in string) (bool, error) { return commit == "6666666666660000" && in == "8888888888880000", nil } - raw, _ := json.Marshal(map[string]any{"revision": 15, "group": "", "repository": "novox/mesh-catalog", - "heads": map[string]string{"novox/mesh-catalog": "8888888888880000"}}) - if err := h3.OnDelivered(stdio.Envelope{Key: DeliveredEvent, Body: raw}); err != nil { - t.Fatal(err) + say := func(rev int, notYet ...string) { + raw, _ := json.Marshal(map[string]any{"revision": rev, "group": "", "repository": "novox/mesh-catalog", + "heads": map[string]string{"novox/mesh-catalog": "8888888888880000"}, "not_yet": notYet}) + if err := h3.OnDelivered(stdio.Envelope{Key: DeliveredEvent, Body: raw}); err != nil { + t.Fatal(err) + } } + // A merge the event says is not yet on the machines (its module runs on no node yet) waits for the next event. + say(15, "6666666666660000") + if v, _ := h3.View(reached); v.Delivery != "fix merged, not delivered: mesh-catalog#34" { + t.Errorf("a merge said not yet: %q", v.Delivery) + } + say(15) for n, want := range map[uint64]string{reached: "delivered to every machine it should reach", later: "fix merged, not delivered: mesh-catalog#35"} { if v, _ := h3.View(n); v.Delivery != want { t.Errorf("per repository: issue %d reads %q, wanted %q", n, v.Delivery, want) diff --git a/modules/mesh-issues/cmd/mesh-issues/delivery.go b/modules/mesh-issues/cmd/mesh-issues/delivery.go index bd21bcfc..75812f9f 100644 --- a/modules/mesh-issues/cmd/mesh-issues/delivery.go +++ b/modules/mesh-issues/cmd/mesh-issues/delivery.go @@ -438,6 +438,9 @@ func (t *Tracker) OnDelivered(e stdio.Envelope) error { Heads map[string]string `json:"heads"` // At is when the view judged the revision delivered, when it says so; else this tracker's moment of hearing it. At time.Time `json:"at"` + // NotYet are, in the per-repository form, merges at or before the head that are not yet on the machines (a module a node + // wants and runs nothing countable of, ADR 0556 clause 2): a fix merged as one of them waits for a later event. + NotYet []string `json:"not_yet"` } if err := json.Unmarshal(e.Body, &body); err != nil || (body.Group == "" && len(body.Heads) == 0) { return nil @@ -457,7 +460,8 @@ func (t *Tracker) OnDelivered(e stdio.Envelope) error { if f != nil && f.Merged && !contains(is.Delivered, pr) && f.MergedAs == "" { t.fillMerge(n, f, pr) } - if f == nil || !f.Merged || contains(is.Delivered, pr) || !(deliveredBy(f, body.Group, body.Heads) || t.coveredBy(f, body.Heads)) { + if f == nil || !f.Merged || contains(is.Delivered, pr) || notYet(f, body.NotYet) || + !(deliveredBy(f, body.Group, body.Heads) || t.coveredBy(f, body.Heads)) { continue } pr := pr @@ -585,6 +589,16 @@ func deliveredBy(f *Fix, group string, heads map[string]string) bool { return false } +// notYet is whether a fix's merge is among the merges an event says are not yet on the machines. +func notYet(f *Fix, merges []string) bool { + for _, m := range merges { + if f.MergedAs != "" && sameCommitIn(f.MergedAs, m) { + return true + } + } + return false +} + // sameCommitIn is whether two commits are one, however either is abbreviated (at least seven characters). func sameCommitIn(a, b string) bool { a, b = strings.ToLower(a), strings.ToLower(b)