diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/fakes_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/fakes_test.go index c5627690..2e1d3c87 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/fakes_test.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/fakes_test.go @@ -128,6 +128,19 @@ type fakeController struct { planned *DeliveryPlan // forgot are the pull requests whose check builds were let go of (novox/hq ADR 0350 §5), "owner/repo#n". forgot []string + // queue is the build queue as Check fills it: every ask made is held, waiting, until a test takes it out (a lost + // ask, issue 1274); queueDown makes the queue unreadable. + queue []QueuedAsk + queueDown bool +} + +func (c *fakeController) Queue() ([]QueuedAsk, error) { + c.mu.Lock() + defer c.mu.Unlock() + if c.queueDown { + return nil, errors.New("the queue cannot be read") + } + return append([]QueuedAsk(nil), c.queue...), nil } // BackupsAgain records the word, and clears the walk's failed backups as the controller does. @@ -199,6 +212,7 @@ func (c *fakeController) Check(group string, ms []Member) (string, error) { } c.checks = append(c.checks, group+":"+strings.Join(heads, ",")) for _, m := range ms { + c.queue = append(c.queue, QueuedAsk{ID: id, State: "waiting", Kind: "check", Repository: m.Repository, Commit: m.Head}) c.checkReady = append(c.checkReady, m.Ready) c.baseSHAs = append(c.baseSHAs, m.BaseSHA) } diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go index 301ed7ed..fa731a8e 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go @@ -71,6 +71,8 @@ type Holder struct { unreached string // askTried is when a check this owner asks was last tried and could not be asked: tried again a minute on. askTried map[string]time.Time + // lostRead is when the build queue was last read for a proposed delivery's asked check (issue 1274). + lostRead map[string]time.Time // walkAsked is when a walk that carried a step still judging was last asked for by its id: once an hour. walkAsked map[string]time.Time refusals []string @@ -165,6 +167,7 @@ func (h *Holder) init() { h.unmatched = map[string]unmatched{} h.waitsWhy = map[string]string{} h.askTried = map[string]time.Time{} + h.lostRead = map[string]time.Time{} h.walkAsked = map[string]time.Time{} h.unchecked = map[string]unchecked{} h.walks = map[string]seenWalk{} @@ -1392,6 +1395,10 @@ func (h *Holder) plan() []func() { if a := h.askChecks(now); a != nil { asks = append(asks, a) } + // Proposed, its check asked and held by no entry of the build queue past the bound: asked again (issue 1274). + if a := h.lostChecks(now); a != nil { + asks = append(asks, a) + } // Merged with no walk yet: tied to the newest walk heard that answers it — by name, by its own commit, or by // the forge's word that the walk's commit contains it, asked here (novox/hq issue 362). diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1274_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1274_test.go new file mode 100644 index 00000000..170452c7 --- /dev/null +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1274_test.go @@ -0,0 +1,153 @@ +package main + +import ( + "strings" + "testing" + "time" +) + +// Issue 1274: a proposed delivery whose asked check the build queue no longer holds is asked again, and a recheck from +// proposed is taken when no check of its head is waiting or in flight. + +// 1274/AC1: the queue holds nothing for the ask, the sweep runs after the bound: the check is asked once more and the +// transition says the earlier ask was lost. +func TestACheckTheQueueLostIsAskedAgainBySweepAfterTheBound(t *testing.T) { + w := newWorld(t) + id := IDOf("novox/app", head) + w.h.PullUpdated(pr("novox/app", 7, head, "feat/x", "src/a.go")) + w.settleAll() + first := w.delivery(id).CheckAsk + if len(w.ctl.checks) != 1 || first == nil { + t.Fatalf("asked %v, ask %+v", w.ctl.checks, first) + } + w.ctl.queue = nil // the queue lost it + w.later(9 * time.Minute) + w.settleAll() + if len(w.ctl.checks) != 1 { + t.Fatalf("asked again inside the bound: %v", w.ctl.checks) + } + w.later(2 * time.Minute) + w.settleAll() + d := w.delivery(id) + last := d.Transitions[len(d.Transitions)-1] + if len(w.ctl.checks) != 2 || d.CheckAsk == nil || d.CheckAsk.ID == first.ID || last.Event != EvLost || + !strings.Contains(last.Why, first.ID) || !strings.Contains(last.Why, "lost") { + t.Fatalf("asked %v; ask %+v; last transition %+v", w.ctl.checks, d.CheckAsk, last) + } + w.later(time.Hour) + w.settleAll() + if len(w.ctl.checks) != 2 { + t.Fatalf("an ask the queue holds was asked again: %v", w.ctl.checks) + } +} + +// 1274/AC1: a verdict that went out of date, its check against the trunk's head asked and lost, is asked again too (the +// case of novox/mesh-controller@28a278c1462a), and the ask's bound runs from the new ask. +func TestACheckAgainstTheTrunkTheQueueLostIsAskedAgain(t *testing.T) { + w := approvedGroup(t) + id := IDOf(gCtl, gHc) + w.h.PullMerged(merged(gCtl, 5, "5555555555555555", gT1)) + w.settleAll() + d := w.delivery(id) + if d.State != Proposed || d.AskedAgain == nil { + t.Fatalf("it is %s, asked again %v", d.State, d.AskedAgain) + } + asked := len(w.ctl.checks) + w.ctl.queue = nil + w.later(11 * time.Minute) + w.settleAll() + d = w.delivery(id) + last := d.Transitions[len(d.Transitions)-1] + if len(w.ctl.checks) != asked+1 || last.Event != EvLost || d.AskedAgain == nil || !d.AskedAgain.After(w.now.Add(-time.Minute)) { + t.Fatalf("asked %v; last transition %+v; asked again %v", w.ctl.checks[asked:], last, d.AskedAgain) + } +} + +// 1274/AC2: the queue still holds the ask, waiting or in flight, or cannot be read: nothing is asked again. +func TestACheckTheQueueStillHoldsIsNotAskedAgain(t *testing.T) { + w := newWorld(t) + id := IDOf("novox/app", head) + w.h.PullUpdated(pr("novox/app", 7, head, "feat/x", "src/a.go")) + w.settleAll() + w.ctl.queue[0].State = "in flight" + w.later(3 * time.Hour) + w.settleAll() + if len(w.ctl.checks) != 1 { + t.Fatalf("an ask in flight was asked again: %v", w.ctl.checks) + } + w.ctl.queue = nil + w.ctl.queueDown = true + w.later(time.Hour) + w.settleAll() + if len(w.ctl.checks) != 1 || w.delivery(id).State != Proposed { + t.Fatalf("an unreadable queue asked again: %v", w.ctl.checks) + } + w.ctl.queueDown = false + w.later(3 * time.Minute) + w.ctl.queue = []QueuedAsk{{ID: "elsewhere", State: "dead", Kind: "check", Repository: "novox/app", Commit: head}} + w.settleAll() + if len(w.ctl.checks) != 2 { + t.Fatalf("a dead ask counted as held: %v", w.ctl.checks) + } +} + +// 1274/AC3: recheck of a proposed delivery with no check waiting or in flight is taken and asks its check; with one +// held it is refused naming it. +func TestARecheckFromProposedIsTakenOnlyWhenNoCheckOfItsHeadIsHeld(t *testing.T) { + w := newWorld(t) + id := IDOf("novox/app", head) + w.h.PullUpdated(pr("novox/app", 7, head, "feat/x", "src/a.go")) + w.settleAll() + held := w.delivery(id).CheckAsk.ID + if _, err := w.h.Recheck(id, "nothing came", "jochen"); err == nil || !strings.Contains(err.Error(), held) { + t.Fatalf("a recheck with a check waiting: %v", err) + } + if _, err := w.h.Recheck(id, "", "jochen"); err == nil { + t.Fatal("a recheck with no why was taken") + } + if len(w.ctl.checks) != 1 { + t.Fatalf("a refused recheck asked: %v", w.ctl.checks) + } + w.ctl.queue = nil + said, err := w.h.Recheck(id, "nothing came", "jochen") + d := w.delivery(id) + last := d.Transitions[len(d.Transitions)-1] + if err != nil || len(w.ctl.checks) != 2 || d.State != Proposed || last.Event != EvLost || last.By != "jochen" || + !strings.Contains(last.Why, "nothing came") || d.CheckAsk.ID != "check-2" || !strings.Contains(said, "check-2") { + t.Fatalf("%q %v; asked %v; last transition %+v", said, err, w.ctl.checks, last) + } + // Taken, it is now held: a second recheck is refused. + if _, err := w.h.Recheck(id, "again", "jochen"); err == nil || !strings.Contains(err.Error(), "check-2") { + t.Fatalf("a second recheck: %v", err) + } +} + +// 1274/AC4: a group member re-asked this way has its group's composed check asked again at the members' heads once its +// own check answers. +func TestAGroupMemberReaskedAfterALostCheckHasTheComposedCheckAskedAgain(t *testing.T) { + w := approvedGroup(t) + id := IDOf(gCat, gHk) + if _, err := w.h.Recheck(id, "its verdict is wanted again", "jochen"); err != nil { + t.Fatal(err) + } + w.settleAll() // its own check is recorded as asked + w.h.mu.Lock() + w.h.groups["feat/g"].Check.OutOfDate = "" // as if the composed check stood: only the lost ask's own act marks it + w.h.mu.Unlock() + w.ctl.queue = nil + w.later(11 * time.Minute) + w.settleAll() + g := w.h.groups["feat/g"] + if g.Check.OutOfDate == "" || !strings.Contains(g.Check.OutOfDate, "lost") { + t.Fatalf("the composed check stands: %+v", g.Check) + } + first := g.Check.Asked + asked := len(w.ctl.checks) + w.later(time.Minute) + w.h.Checked(judged(gCat, 2, gHk, "pass", gTrunk0)) + w.settleAll() + last := w.ctl.checks[len(w.ctl.checks)-1] + if len(w.ctl.checks) <= asked || !strings.HasPrefix(last, "feat/g:") || w.h.groups["feat/g"].Check.Asked == first { + t.Fatalf("the composed check was not asked again: %v", w.ctl.checks[asked:]) + } +} diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1349_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1349_test.go index bb282ec2..195bef45 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1349_test.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1349_test.go @@ -97,7 +97,7 @@ func TestTheGatesCodeCallsNoForgeWriteToolAndNoDeliveryPlan(t *testing.T) { forbidden := []string{"delivery-plan", "gitea_commit_status\"", "gitea_delivery_view", "gitea_note_append", "gitea_add_comment", "gitea_merge_pull_request\"", "gitea_open_pulls_write"} // The controller's verbs the gate calls: ADR 0450 §7. - allowed := []string{"deliver", "delivery-stop", "delivery-backups-again", "delivery-walks", "delivery-check", "forget-checks", "plans"} + allowed := []string{"deliver", "delivery-stop", "delivery-backups-again", "delivery-walks", "delivery-check", "forget-checks", "plans", "queue"} called := map[string]bool{} fset := token.NewFileSet() for _, f := range files { diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/lostcheck.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/lostcheck.go new file mode 100644 index 00000000..07725fa8 --- /dev/null +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/lostcheck.go @@ -0,0 +1,211 @@ +package main + +import ( + "fmt" + "strings" + "time" +) + +// A check the build queue lost (novox/hq issue 1274). +// +// A proposed delivery waits for a check it asked of the controller. When the build queue loses that ask — taken by a +// build agent that was replaced, or dropped with the queue's own state — nothing answers and nothing asks again: the +// delivery stays proposed until a new head is pushed, and a recheck from proposed was refused. The sweep now reads the +// controller's `queue` and asks again a check that no entry holds (neither waiting nor in flight; a dead one is not +// held) for longer than lostCheckBound. A recheck of a proposed delivery is taken on the same fact. Both are the +// table's EvLost, said in the delivery's transitions; neither asks while the queue holds an ask of the head, so no +// ask is made twice. A queue that cannot be read asks nothing: a guess would risk a duplicate. + +// lostReadEvery is how often the queue is read for one proposed delivery's ask: the read is one bus call, and a check +// the queue holds is looked at again no sooner. +const lostReadEvery = 2 * time.Minute + +// lostCheckBound is how long a proposed delivery's asked check may go with no entry in the build queue before it is +// asked again: the repository's last measured check time (ADR 0505 §4, lastCheckTime), at least UncheckedAfter (ten +// minutes, the bound a check never asked is said at). Under the lock. +func (h *Holder) lostCheckBound(repository string) time.Duration { + return max(UncheckedAfter, h.lastCheckTime(repository)) +} + +// awaitedSince is when a proposed delivery's check was asked, when it waits for one this owner asked: no verdict yet, +// or a verdict out of date with its check against the trunk's head asked. False for a delivery waiting for none. +func awaitedSince(d *Delivery) (time.Time, bool) { + if d.State != Proposed || d.MergedAs != "" || d.Number == 0 { + return time.Time{}, false + } + switch { + case d.Check == nil && askedCheck(d): + return d.CheckAsk.At, true + case d.Check != nil && d.outOfDate() != "" && d.AskedAgain != nil: + return *d.AskedAgain, true + } + return time.Time{}, false +} + +// heldBy is the queue's entry that holds a check of the delivery's head: its own ask by id, or any check of its +// repository at its head — a group's composed check included, which a second ask would only repeat. +func heldBy(d *Delivery, asks []QueuedAsk) *QueuedAsk { + for i, a := range asks { + if !a.Holds() { + continue + } + if d.CheckAsk != nil && d.CheckAsk.ID != "" && a.ID == d.CheckAsk.ID { + return &asks[i] + } + if a.Kind == "check" && a.Commit != "" && sameCommit(a.Commit, d.Commit) && + strings.Contains(strings.ToLower(a.Repository), strings.ToLower(d.Repository)) { + return &asks[i] + } + } + return nil +} + +func (a QueuedAsk) words() string { + // The controller's `queue` says the state "in flight", its table's word. + state := strings.ReplaceAll(a.State, "-", " ") + if state == "in flight" && a.Machine != "" { + return a.ID + " is in flight on " + a.Machine + } + return a.ID + " is " + state + " in the build queue" +} + +// lostChecks are the asks again of proposed deliveries whose asked check the build queue holds no entry of, past the +// bound, at each look. Under the lock; the queue is read, and the asks made, outside it. +func (h *Holder) lostChecks(now time.Time) func() { + var ids []string + for id, d := range h.deliveries { + since, ok := awaitedSince(d) + if !ok { + delete(h.lostRead, id) + continue + } + due := since.Add(h.lostCheckBound(d.Repository)) + if now.Before(due) { + h.lookAt(due, "a proposed delivery's asked check, read in the build queue once its bound passed") + continue + } + if read, was := h.lostRead[id]; was && now.Sub(read) < lostReadEvery { + h.lookAt(read.Add(lostReadEvery), "a proposed delivery's asked check, looked for in the build queue again") + continue + } + h.lostRead[id] = now + h.lookAt(now.Add(lostReadEvery), "a proposed delivery's asked check, looked for in the build queue again") + ids = append(ids, id) + } + if len(ids) == 0 { + return nil + } + return func() { + asks, err := h.Controller.Queue() + if err != nil { + h.Logf("[mesh-delivery] the build queue cannot be read, so no check is asked again for a lost ask: %v", err) + return + } + for _, id := range ids { + h.mu.Lock() + d := h.deliveries[id] + since, ok := time.Time{}, false + if d != nil { + since, ok = awaitedSince(d) + } + var held *QueuedAsk + if ok { + held = heldBy(d, asks) + } + var why string + if ok && held == nil { + what := "its check against the trunk's head" + if d.Check == nil { + what = "its check " + d.CheckAsk.ID + } + why = fmt.Sprintf("%s, asked %s ago, is held by no entry of the build queue (neither waiting nor in flight): "+ + "lost, asked again", what, h.Now().Sub(since).Round(time.Second)) + } + h.mu.Unlock() + if why == "" { + continue + } + if _, err := h.askLost(id, byOwner, why, asks); err != nil { + h.Logf("[mesh-delivery] %s: its lost check could not be asked again: %v", id, err) + h.again("a proposed delivery's lost check that could not be asked again") + } + } + } +} + +// recheckProposed is `recheck` of a proposed delivery (issue 1274): taken, with why, when no check of its head is +// waiting or in flight in the build queue, and refused naming the one that is. A queue that cannot be read refuses it. +func (h *Holder) recheckProposed(id, why, by string) (string, error) { + if why == "" { + return "", fmt.Errorf("a recheck says why") + } + asks, err := h.Controller.Queue() + if err != nil { + return "", fmt.Errorf("%s is not rechecked: the build queue cannot be read (%v), so whether its check is still held is not known", id, err) + } + h.mu.Lock() + d := h.deliveries[id] + var held *QueuedAsk + if d != nil { + held = heldBy(d, asks) + } + h.mu.Unlock() + if held != nil { + return "", fmt.Errorf("%s is not rechecked: its check is still held, %s", id, held.words()) + } + return h.askLost(id, by, why, asks) +} + +// askLost asks a proposed delivery's check again, the table's EvLost: tried first on a copy, so an ask the controller +// does not take spends nothing. Its group's composed check goes out of date, and is asked once the member answers. +func (h *Holder) askLost(id, by, why string, asks []QueuedAsk) (string, error) { + h.mu.Lock() + d := h.deliveries[id] + if d == nil { + h.mu.Unlock() + return "", fmt.Errorf("no delivery %q", id) + } + if held := heldBy(d, asks); held != nil { + h.mu.Unlock() + return "", fmt.Errorf("%s is not asked again: its check is still held, %s", id, held.words()) + } + probe := *d + probe.Transitions = nil + if _, err := Apply(&probe, EvLost, true, Facts{Now: h.Now(), Why: why, By: by}); err != nil { + h.mu.Unlock() + return "", err + } + m := MemberOf(d) + if d.Check != nil && d.outOfDate() != "" && d.TrunkNow != "" { + m.BaseSHA = d.TrunkNow + } + h.mu.Unlock() + asked, err := h.Controller.Check("", []Member{m}) + if err != nil { + return "", fmt.Errorf("%s's check could not be asked again yet: %w", id, err) + } + h.mu.Lock() + defer h.mu.Unlock() + d = h.deliveries[id] + if d == nil || d.State != Proposed { + return "", fmt.Errorf("%s moved while its check was asked again", id) + } + now := h.Now() + t, err := Apply(d, EvLost, true, Facts{Now: now, Why: why + " — asked as " + asked, By: by}) + if err != nil { + return "", err + } + d.CheckAsk = &CheckAsk{ID: asked, At: now, By: by, Why: why} + if d.Check != nil { + d.AskedAgain, d.FreshWanted = &now, false + } + delete(h.askTried, id) + if g := h.groups[d.Group]; g != nil && g.Check != nil { + g.Check.OutOfDate = fmt.Sprintf("composed check out of date: %s's check was lost in the build queue and asked again", d.ID) + g.Check.OutOfDateAt = now + g.Updated = now + h.keepGroup(g) + } + h.moved(d, []Transition{t}) + return fmt.Sprintf("%s: its check was held by no entry of the build queue and is asked again as %s (%s)", id, asked, why), nil +} diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/manifest_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/manifest_test.go index b1ca5f0f..4a9c41bc 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/manifest_test.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/manifest_test.go @@ -92,7 +92,7 @@ func TestItClaimsTheDeliverySeatAndFollowsWhatItHandles(t *testing.T) { // It calls exactly the controller's seven verbs (ADR 0450 §7) and reads the forge with three tools; the forge's // writes are the bridge's (ADR 0437 §8). It uses no operator-channel: the teller asks (ADR 0437 §6). want := []string{} - for _, v := range []string{"deliver", "delivery-stop", "delivery-backups-again", "delivery-walks", "delivery-check", "forget-checks", "plans"} { + for _, v := range []string{"deliver", "delivery-stop", "delivery-backups-again", "delivery-walks", "delivery-check", "forget-checks", "queue", "plans"} { want = append(want, "seat:"+ControllerSeat+"."+v) } want = append(want, "gitea.gitea_commit_statuses", "gitea.gitea_contains", "gitea.gitea_open_pulls") diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ports.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ports.go index 4cb8767f..c2af77b7 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ports.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ports.go @@ -67,8 +67,24 @@ type Controller interface { Walks(walk string) (bool, []Walk, error) // ForgetChecks lets go of the builds a pull request's merge checks made for its merge (novox/hq ADR 0350 §5). ForgetChecks(owner, repo string, number int) error + // Queue is every ask the build queue holds, waiting or in flight (the controller's `queue`, novox/hq issue 455): what + // tells a check still on its way from one the queue lost (issue 1274). A dead ask is not held. + Queue() ([]QueuedAsk, error) } +// QueuedAsk is one ask of the build queue as the controller's `queue` answers it as data. +type QueuedAsk struct { + ID string `json:"id"` + State string `json:"state"` // waiting, in flight or dead + Kind string `json:"kind"` // check or delivery + Repository string `json:"repository"` + Commit string `json:"commit,omitempty"` + Machine string `json:"machine,omitempty"` +} + +// Holds is whether the ask is still the queue's to run: waiting or in flight, never dead. +func (q QueuedAsk) Holds() bool { return q.State != "dead" } + // Forge is what the forge's holder does for a delivery. type Forge interface { // Statuses reads a commit's statuses — a pull request's head's, given its number — and whether the branch @@ -250,6 +266,20 @@ func (c seatController) Check(group string, members []Member) (string, error) { return "", fmt.Errorf("the controller's delivery-check answered no check id (%s): it is not taken as asked", clip(string(raw), 200)) } +func (c seatController) Queue() ([]QueuedAsk, error) { + raw, err := c.verb("queue", map[string]any{}) + if err != nil { + return nil, err + } + var a struct { + Asks []QueuedAsk `json:"asks"` + } + if err := json.Unmarshal(raw, &a); err != nil { + return nil, fmt.Errorf("the controller's queue is not readable: %w", err) + } + return a.Asks, nil +} + func (c seatController) Deliver(walk, why string) error { _, err := c.verb("deliver", map[string]any{"plan": walk, "why": why}) return err diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/table.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/table.go index b8ed6d08..96826ae5 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/table.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/table.go @@ -69,6 +69,7 @@ const ( EvRefused Event = "refused" // the verdict fails, or the check could not run EvRecheck Event = "recheck" // a person asked for it again, or its group changed EvReask Event = "re-ask" // healer H2 asked a check asked and not answered once more + EvLost Event = "check lost" // the build queue holds no check of its head and it was asked: asked again (issue 1274) EvNewHead Event = "new-head" // a newer head of the same pull request EvClosed Event = "closed" // the pull request closed unmerged EvMerged Event = "merged" // it reached the trunk @@ -240,6 +241,24 @@ var Table = []Row{ return nil }}, + // **A check the build queue lost is asked again** (novox/hq issue 1274): a proposed delivery waits for a check, and + // the queue, which the table cannot read, holds none of its head. The act (the sweep's or a person's recheck) reads + // the queue first and says why; the table keeps it to a delivery that waits for a verdict. + {From: []State{Proposed}, Event: EvLost, To: Proposed, Act: true, + Guard: "it waits for a check — no verdict yet, or one out of date — nothing of its head is waiting or in flight in " + + "the build queue, and who asks says why", + Holds: func(d *Delivery, f Facts) error { + switch { + case f.Why == "" || f.By == "": + return errors.New("a lost check's ask says who and why") + case d.MergedAs != "": + return errors.New("it is merged") + case d.Check != nil && d.outOfDate() == "": + return errors.New("its verdict is known") + } + return nil + }}, + {From: []State{Published}, Event: EvSuperseded, To: Superseded, Guard: "its walk was superseded and names no walk that took it over: a walk that took it over answers its merge, and it follows that walk (novox/hq ADR 0276)", Holds: func(d *Delivery, f Facts) error { diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/table_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/table_test.go index 573e4935..87f5b5c9 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/table_test.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/table_test.go @@ -43,7 +43,7 @@ func aDeliveryFor(r Row) (*Delivery, Facts) { d.Check = &Verdict{Gate: "pass"} case EvRefused: d.Check = &Verdict{Gate: "fail"} - case EvRecheck, EvRelease, EvStop: + case EvRecheck, EvRelease, EvStop, EvLost: f.Why, f.By = "a person's reason", "jochen" if r.Event == EvStop && !r.Act { f = Facts{Now: now} @@ -133,7 +133,7 @@ func TestEveryRowOfTheTableIsTakenWhenItsGuardHoldsAndRefusedWhenNot(t *testing. } func TestEveryPairTheTableDoesNotHoldIsRefusedByName(t *testing.T) { - events := []Event{EvAnnounced, EvAppeared, EvAdopted, EvChecked, EvAccepted, EvRefused, EvRecheck, EvReask, EvNewHead, + events := []Event{EvAnnounced, EvAppeared, EvAdopted, EvChecked, EvAccepted, EvRefused, EvRecheck, EvReask, EvLost, EvNewHead, EvClosed, EvMerged, EvMergedUnchecked, EvGo, EvHold, EvRelease, EvDone, EvFailed, EvSuperseded, EvStop, EvHistory, EvRetire, EvCovered, EvLifted, EvRewalk, EvTrunkMoved, EvBackupFailed} for _, from := range append([]State{None}, AllStates...) { diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/verbs.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/verbs.go index b20ecf9a..27e876c5 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/verbs.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/verbs.go @@ -303,6 +303,11 @@ func (h *Holder) Recheck(id, why, by string) (string, error) { h.mu.Unlock() return "", fmt.Errorf("no delivery %q", id) } + if d.State == Proposed { + // A proposed delivery's check is asked again when the build queue holds none of its head (issue 1274). + h.mu.Unlock() + return h.recheckProposed(id, why, by) + } // Whether the table takes it, tried on a copy: nothing is said on the forge for a recheck it refuses. probe := *d probe.Transitions = append([]Transition(nil), d.Transitions...) diff --git a/modules/mesh-delivery-gate/module.json b/modules/mesh-delivery-gate/module.json index d3faa1dd..b9a06d0e 100644 --- a/modules/mesh-delivery-gate/module.json +++ b/modules/mesh-delivery-gate/module.json @@ -47,6 +47,7 @@ "seat:mesh-controller.delivery-walks", "seat:mesh-controller.delivery-check", "seat:mesh-controller.forget-checks", + "seat:mesh-controller.queue", "seat:mesh-controller.plans", "gitea.gitea_commit_statuses", "gitea.gitea_contains",