From fdb8deea4c9e787c7a550443fb4bfe4071af4fab Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 31 Oct 2026 02:16:08 +0100 Subject: [PATCH] Gate: an ask ends once no delivery it covers can still merge by it, so an ask of a pull request merged another way cannot hold the teller's room for a day (issue 774) endStaleAsks skipped every merged delivery, so a pull request merged without the Approve kept its merge ask in the gate's asks until it lapsed: on 2026-10-31 two such asks held two of the router's three places for the teller and a placement ask was refused for want of room. Every look now takes away a standing ask none of whose deliveries is still open, read from what the gate keeps, so a failed take away is retried. A settle asked again for another set takes the earlier settle ask away for the same reason. --- .../cmd/mesh-delivery-gate/approval.go | 60 ++++++++++++++++++- .../cmd/mesh-delivery-gate/approval_test.go | 21 +++++++ .../cmd/mesh-delivery-gate/settle.go | 20 +++++++ .../cmd/mesh-delivery-gate/settle_test.go | 14 +++++ 4 files changed, 114 insertions(+), 1 deletion(-) 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 bfbcfae4..c635afc6 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval.go @@ -325,14 +325,17 @@ func (h *Holder) headsHold(d *Delivery, c *ReadyRun) bool { // endStaleAsks ends every ask whose heads are no longer its group's: a push to any member ends the approval (ADR 0398 // §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. +// A Decline stands, and is not ended. An ask none of whose deliveries can still merge by it is taken away after +// (endAsksOfFinished). Under the lock; the cancels run with the look's asks. func (h *Holder) endStaleAsks() { h.endStaleSettles() + ending := map[string]bool{} for _, d := range h.deliveries { r := d.Ready if r == nil || r.Merged != nil || d.MergedAs != "" || r.declined() || (r.AskID == "" && r.Approved == nil) || len(r.Heads) == 0 { continue } + id, before := r.AskID, len(h.cancels) switch { case d.State.Final(): h.cancels = append(h.cancels, h.dropAsk(r.AskID, "its head "+shortOf(d.Commit, 12)+" is no longer the pull request's head, or the pull request closed")) @@ -347,6 +350,61 @@ func (h *Holder) endStaleAsks() { // written again, which the teller asks afresh (ADR 0441 §1). h.cancels = append(h.cancels, h.withdrawAsk(r.AskID, "nobody answered in "+AskLasts.String())) } + if id != "" && (len(h.cancels) > before || h.withdrawn[id]) { + ending[id] = true + } + } + h.endAsksOfFinished(ending) +} + +// endAsksOfFinished takes away every ask that stands in the gate's asks while none of the deliveries it was asked for can still +// be merged by it: each merged (on the operator's Approve or by another path) or ended. The ask is about a merge, so it ends +// once that merge cannot follow from it, however the deliveries got there (issue 774: on 2026-10-31 two pull requests the lead +// merged once their checks were green left their asks standing for a day, and the teller's asks held two of the router's three +// places, so a newer ask was refused for want of room). It reads what the gate keeps (AskedKey, Written), so an ask whose +// taking away failed is taken away again at the next look. An ask another end of this look takes away (ending, withdrawn) is +// left to it. Nothing is said on a pull request: its merge or its end said it. Under the lock. +func (h *Holder) endAsksOfFinished(ending map[string]bool) { + standing, live := map[string]bool{}, map[string]bool{} + for _, d := range h.deliveries { + r := d.Ready + if r == nil || r.AskedKey == "" { + continue + } + if r.Written { + standing[r.AskedKey] = true + } + if d.MergedAs == "" && !d.State.Final() { + live[r.AskedKey] = true + } + } + keys := make([]string, 0, len(standing)) + for key := range standing { + if !live[key] && !ending[key] && !h.withdrawn[key] { + keys = append(keys, key) + } + } + sort.Strings(keys) + for _, key := range keys { + for _, d := range h.deliveries { + if r := d.Ready; r != nil && r.AskID == key && r.Approved == nil { + r.AskID, r.Ask, r.Heads = "", nil, nil + h.keep(d) + } + } + h.Logf("[mesh-delivery] the operator's ask %s is taken away: every pull request it was asked for is merged or ended", key) + if h.Operator == nil { + continue + } + h.cancels = append(h.cancels, func() { + if err := h.Operator.Cancel(key); err != nil { + h.Logf("[mesh-delivery] the operator's ask %s could not be taken away, and is taken away at the next look (%v)", key, err) + return + } + h.mu.Lock() + h.markWritten(key, false) + h.mu.Unlock() + }) } } diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval_test.go index 3057f3d8..9019225d 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval_test.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval_test.go @@ -160,6 +160,27 @@ func TestAPushAfterTheAskCancelsItAndTheNewHeadIsAskedOnItsOwnWhenItsChecksPass( if open := w.op.open(); len(open) != 1 || open[0] == old { t.Fatalf("the new head was not asked afresh: %v", open) } + // Issue 774 (2026-10-31): a pull request merged by another path than the Approve (the lead merged it once its check was + // green) left its ask in the gate's asks for a day, and the teller's ask held one of the router's three places. An ask whose + // every delivery is merged or ended is taken away at the next look, saying nothing on the pull request. + t.Run("774: an ask whose pull request merged without it is taken away", func(t *testing.T) { + w := markedGreen(t) + ask := w.op.open()[0] + comments := len(w.forge.comments) + w.h.PullMerged(merged("novox/app", 7, readyHead, "m1m1m1m1m1m1m1m1m1m1m1m1m1m1m1m1m1m1m1m1")) + w.settleAll() + if len(w.op.open()) != 0 || len(w.op.cancelled) != 1 || w.op.cancelled[0] != ask { + t.Fatalf("the ask of a merged pull request stands: open %v, taken away %v", w.op.open(), w.op.cancelled) + } + if d := w.delivery(IDOf("novox/app", readyHead)); d.Ready.AskID != "" || d.Ready.Written { + t.Errorf("the merged delivery still keeps its ask: %+v", d.Ready) + } + w.later(2 * time.Minute) + w.settleAll() + if len(w.op.cancelled) != 1 || w.op.count() != 1 || len(w.forge.comments) != comments { + t.Errorf("taken away %v, asks %d, comments %v: once, asking nothing again, saying nothing", w.op.cancelled, w.op.count(), w.forge.comments[comments:]) + } + }) } // The ask lapses at the router after AskLasts with nobody's answer, and the gate has no word of it: it takes its key away and 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 a95ef111..d8d977a6 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/settle.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/settle.go @@ -98,11 +98,31 @@ func (h *Holder) Settle(ids []string, why, by string) (map[string]any, error) { } } ask := sdk.Ask{Group: group, Heads: heads, Asking: asking, Ask: composeSettleAsk(sdk.AskingID(key, asking), covered, why, now)} + earlier := map[string]bool{} for _, d := range covered { + if d.Settle != nil && d.Settle.Answered == nil && d.Settle.Key != key { + earlier[d.Settle.Key] = true + } d.Settle = &SettleAsk{Key: key, Asking: asking, Why: why, By: by, At: now} h.keep(d) } + // An earlier settle ask of another set that no delivery waits for any more is taken away (issue 774): its question is + // gone, and left standing it would hold the teller's room at the router until it lapsed. + for _, d := range h.deliveries { + if d.Settle != nil { + delete(earlier, d.Settle.Key) + } + } + gone := make([]string, 0, len(earlier)) + for k := range earlier { + gone = append(gone, k) + } + sort.Strings(gone) h.mu.Unlock() + for _, k := range gone { + h.Logf("[mesh-delivery] the settle ask %s is taken away: settle asked again for another set, and no delivery waits for it", k) + h.cancelSettle(k)() + } if err := h.Operator.Ask(key, ask); err != nil { return nil, fmt.Errorf("the settle ask %s could not be written (call settle again): %w", key, err) } 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 bbe88fae..f4b31dd6 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 @@ -1,6 +1,7 @@ package main import ( + "slices" "strings" "testing" "time" @@ -110,4 +111,17 @@ func TestSettleAsksTheOperatorOnceAndMovesOnlyOnTheAnswer(t *testing.T) { if c.State != Held || c.Settle.Answer != "lapsed" { t.Errorf("a lapsed settle ask: %s %+v", c.State, c.Settle) } + // Issue 774: a settle asked again for another set of deliveries is another key; the earlier ask, which no delivery waits + // for any more, is taken away at once, not left to hold the teller's room at the router until it lapses. + e, f := put("novox/web@eeeeeeeeeeee", Held, HeldUnchecked), put("novox/web@ffffffffffff", Held, HeldUnchecked) + first, err := w.h.Settle([]string{e.ID}, "one", aPerson) + if err != nil { + t.Fatal(err) + } + if _, err := w.h.Settle([]string{e.ID, f.ID}, "both", aPerson); err != nil { + t.Fatal(err) + } + if k := first["asked"].(string); !slices.Contains(w.op.cancelled, k) || slices.Contains(w.op.open(), k) { + t.Errorf("the earlier settle ask %s stands once no delivery waits for it: open %v, taken away %v", k, w.op.open(), w.op.cancelled) + } }