Merge pull request 'Ask the check of a delivery that waits for one nobody asked (hq issue 290)' (#101) from fix/a-delivery-waiting-gets-its-check into main

This commit was merged in pull request #101.
This commit is contained in:
2026-10-07 11:20:18 +00:00
6 changed files with 359 additions and 15 deletions
@@ -0,0 +1,170 @@
package main
import (
"strings"
"testing"
"time"
)
// A delivery that waits for its check gets one (novox/hq issue 290): a head adopted while it already waited —
// its announcement heard again from the bus's history, which the controller took long ago — has its check asked
// by this owner; so does one whose announcement the controller missed. Once asked, the proposed bound means
// "asked and not answered", and healer H2 may ask it once more before it is the operator's.
// The live case: a pull request opened before merge checks existed, announced again at this owner's first
// start, read back proposed after a restart. Its check is asked at the first tick, once, for that head alone.
func TestAProposedDeliveryAdoptedWithNoVerdictHasItsCheckAsked(t *testing.T) {
w := newWorld(t)
id := IDOf("novox/hq", head)
w.h.PullUpdated(pr("novox/hq", 94, head, "issues/245-246", "04-ISSUES/245/00-report.md"))
w.settleAll()
if len(w.ctl.checks) != 0 {
t.Fatalf("a head just announced was asked at once, before the controller's own check had time: %v", w.ctl.checks)
}
// A restart, hours later: read back proposed, no verdict, nothing asked.
again := newWorldOver(t, w.store, w.ctl, w.forge, w.now.Add(10*time.Hour))
if s := again.state(id); s != Proposed {
t.Fatalf("read back, it is %s", s)
}
again.settleAll()
if len(w.ctl.checks) != 1 || w.ctl.checks[0] != ":"+id {
t.Fatalf("its check was asked %v, wanted once for its head alone", w.ctl.checks)
}
d := again.delivery(id)
if d.CheckAsk == nil || d.CheckAsk.ID != "check-1" || d.CheckAsk.By != byOwner || d.State != Proposed {
t.Fatalf("the ask is not kept: %+v (%s)", d.CheckAsk, d.State)
}
if notes := w.forge.notesOn(head); len(notes) == 0 || !strings.Contains(notes[len(notes)-1], "its check asked") {
t.Fatalf("the ask is not noted on the head: %q", notes)
}
// Asked, it is not stalled: the bound runs from the ask.
if st := again.h.Stalled(); len(st) != 0 {
t.Fatalf("a check just asked is stalled: %+v", st)
}
// Asked once: the ticks after do not ask again.
again.later(30 * time.Minute)
again.settleAll()
if len(w.ctl.checks) != 1 {
t.Fatalf("a check asked and in flight was asked again: %v", w.ctl.checks)
}
// The verdict arrives: checked, then ready, by itself.
again.h.Checked(verdict("novox/hq", 94, head, "pass"))
if s := again.state(id); s != Ready {
t.Fatalf("its verdict left it %s", s)
}
}
// The same head adopted at a first start with no restart: the grace passes, and its check is asked.
func TestAMissedAnnouncementHasItsCheckAskedAfterTheGrace(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.later(checkGrace - time.Minute)
w.settleAll()
if len(w.ctl.checks) != 0 {
t.Fatalf("asked within the grace: %v", w.ctl.checks)
}
// The controller does not answer: nothing is kept as asked, and it is tried again a minute on.
w.later(2 * time.Minute)
w.ctl.down = true
w.settleAll()
if d := w.delivery(id); d.CheckAsk != nil {
t.Fatalf("an ask the controller did not take is kept as asked: %+v", d.CheckAsk)
}
w.ctl.down = false
w.settleAll()
if len(w.ctl.checks) != 0 {
t.Fatalf("asked again within the minute: %v", w.ctl.checks)
}
w.later(time.Minute)
w.settleAll()
if len(w.ctl.checks) != 1 || w.delivery(id).CheckAsk == nil {
t.Fatalf("a missed announcement was not asked once the grace passed: %v", w.ctl.checks)
}
// A verdict that arrived in the meantime, or a delivery past its proposal, is never asked.
other := IDOf("novox/app", "bbbbbbbbbbbb2222")
w.h.PullUpdated(pr("novox/app", 8, "bbbbbbbbbbbb2222", "feat/y"))
w.h.Checked(verdict("novox/app", 8, "bbbbbbbbbbbb2222", "fail"))
w.later(time.Hour)
w.settleAll()
if w.state(other) != Rejected || len(w.ctl.checks) != 1 {
t.Fatalf("a head with a verdict was asked: %s %v", w.state(other), w.ctl.checks)
}
}
// An announcement heard again carrying its head's decided verdict takes it; a pending one does not.
func TestAnAnnouncementCarryingItsHeadsVerdictTakesIt(t *testing.T) {
w := newWorld(t)
p := pr("novox/app", 7, head, "feat/x", "src/a.go")
p.HeadChecks = map[string]string{"mesh/merge-gate": "success", "mesh/repo-check": "success"}
w.h.PullUpdated(p)
if s := w.state(IDOf("novox/app", head)); s != Ready {
t.Fatalf("a head whose gate passed on the forge is %s", s)
}
q := pr("novox/app", 8, "bbbbbbbbbbbb2222", "feat/y")
q.HeadChecks = map[string]string{"mesh/merge-gate": "pending"}
w.h.PullUpdated(q)
if s := w.state(IDOf("novox/app", "bbbbbbbbbbbb2222")); s != Proposed {
t.Fatalf("a pending gate on the forge left it %s", s)
}
}
// Asked and not answered past the bound: stalled, and H2's close re-asks once — the second close is refused
// by the table, and the delivery is the operator's.
func TestH2ReasksAStalledCheckOnceThenItIsTheOperators(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.later(checkGrace + time.Minute)
w.settleAll()
if len(w.ctl.checks) != 1 {
t.Fatalf("asked %v", w.ctl.checks)
}
if _, err := w.h.Close(id, "too early"); err == nil {
t.Fatal("re-asked within the bound")
}
w.later(Bounds[Proposed].For + time.Minute)
st := w.h.Stalled()
if len(st) != 1 || st[0].ID != id || !strings.Contains(st[0].H2, "re-ask") {
t.Fatalf("stalled %+v", st)
}
// The controller does not take the re-ask: nothing is spent.
w.ctl.down = true
if _, err := w.h.Close(id, "delivery stalled"); err == nil {
t.Fatal("a re-ask the controller did not take was said done")
}
if d := w.delivery(id); d.CheckAsk.Reasked != 0 {
t.Fatal("a re-ask that was not made was spent")
}
w.ctl.down = false
said, err := w.h.Close(id, "delivery stalled")
if err != nil || !strings.Contains(said, "re-asked") {
t.Fatalf("H2's close: %q %v", said, err)
}
d := w.delivery(id)
if len(w.ctl.checks) != 2 || d.State != Proposed || d.CheckAsk.Reasked != 1 || d.CheckAsk.By != byH2 ||
d.Transitions[len(d.Transitions)-1].Event != EvReask {
t.Fatalf("after the re-ask: %v %s %+v", w.ctl.checks, d.State, d.CheckAsk)
}
if len(w.h.Stalled()) != 0 {
t.Fatal("a check just re-asked is stalled")
}
// Still not answered: stalled again, and the operator's.
w.later(Bounds[Proposed].For + time.Minute)
st = w.h.Stalled()
if len(st) != 1 || !strings.Contains(st[0].H2, "operator's") {
t.Fatalf("stalled after the re-ask %+v", st)
}
if _, err := w.h.Close(id, "delivery stalled"); err == nil || !strings.Contains(err.Error(), "once already") {
t.Fatalf("a second re-ask: %v", err)
}
if len(w.ctl.checks) != 2 {
t.Fatalf("a second re-ask reached the controller: %v", w.ctl.checks)
}
// The verdict, late, still settles it.
w.h.Checked(verdict("novox/app", 7, head, "pass"))
if s := w.state(id); s != Ready {
t.Fatalf("a late verdict left it %s", s)
}
}
@@ -37,7 +37,9 @@ type Holder struct {
held bool
reconciled time.Time
unreached string
refusals []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
refusals []string
}
// unmatched is a walk no delivery is known for yet: its merge not heard, or merged before this owner held.
@@ -76,6 +78,7 @@ func (h *Holder) init() {
h.dirty = map[string]bool{}
h.dirtyGroup = map[string]bool{}
h.unmatched = map[string]unmatched{}
h.askTried = map[string]time.Time{}
h.started = h.Now()
}
@@ -256,10 +259,29 @@ func (h *Holder) PullUpdated(p PullEvent) {
h.refuse(err)
return
}
h.moved(d, []Transition{t})
ts := []Transition{t}
// An announcement heard again from the bus's history may carry its head's verdict already: the forge holds
// the gate's status on that very commit. A decided one is taken; a pending or errored one is asked again.
if v := decidedOnHead(p.HeadChecks, now); v != nil {
d.Check = v
ts = append(ts, settle(d, Facts{Now: now})...)
}
h.moved(d, ts)
h.joinGroup(d)
}
// decidedOnHead is the verdict the forge's statuses on a head hold, when the gate decided one: passed, warned
// or failed. Nil for none, pending, or a check that could not run — those are asked again.
func decidedOnHead(checks map[string]string, now time.Time) *Verdict {
switch checks["mesh/merge-gate"] {
case "success", "warning", "failure":
default:
return nil
}
return &Verdict{At: now, Gate: verdictOfState(checks["mesh/merge-gate"]),
Summary: "the head's mesh/merge-gate, as the forge holds it", Repo: verdictOfState(checks["mesh/repo-check"])}
}
// joinGroup puts a new head in the group its branch name makes with the other repositories' open pull
// requests (ADR 0239 decision 4): one mechanism, the name the feature already has in every repository.
func (h *Holder) joinGroup(d *Delivery) {
@@ -708,6 +730,14 @@ func (h *Holder) plan() []func() {
}
}
// Proposed, with no verdict and no check asked past the grace: its check is asked — a head adopted while
// it already waited, or one whose announcement the controller missed.
for _, d := range h.deliveries {
if a := h.askCheck(d, now); a != nil {
asks = append(asks, a)
}
}
// 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 == "" &&
@@ -749,6 +779,43 @@ func (h *Holder) plan() []func() {
return asks
}
// 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.
func (h *Holder) askCheck(d *Delivery, now time.Time) func() {
if d.State != Proposed || d.Check != nil || d.CheckAsk != nil || d.MergedAs != "" || d.Number == 0 ||
now.Sub(d.Since) < checkGrace {
return nil
}
if tried, was := h.askTried[d.ID]; was && now.Sub(tried) < time.Minute {
return nil
}
h.askTried[d.ID] = now
id, m := d.ID, MemberOf(d)
why := fmt.Sprintf("proposed for %s with no verdict and no check asked: its announcement was heard from the "+
"bus's history, or the controller missed it", now.Sub(d.Since).Round(time.Minute))
return func() {
asked, err := h.Controller.Check("", []Member{m})
h.mu.Lock()
defer h.mu.Unlock()
if err != nil {
h.Logf("[mesh-delivery] %s's check cannot be asked yet: %v", id, err)
return
}
delete(h.askTried, id)
d := h.deliveries[id]
if d == nil || d.State != Proposed || d.Check != nil {
return
}
d.CheckAsk = &CheckAsk{ID: asked, At: h.Now(), By: byOwner, Why: why}
h.Logf("[mesh-delivery] %s: its check asked of the controller as %s — %s", id, asked, why)
d.Owed = append(d.Owed, Effect{Kind: EffectNote, Commit: d.Commit, Since: h.Now(),
Line: oneLine(fmt.Sprintf("%s delivery %s: its check asked by %s as %s: %s", h.Now().UTC().Format(time.RFC3339),
id, byOwner, asked, why))})
h.keep(d)
}
}
// adopt makes a delivery of a walk no pull request is known for: running already — at the switch, or on the
// controller's own path — it is delivering; waiting for a word, it waits for a person.
func (h *Holder) adopt(w Walk) {
@@ -46,6 +46,9 @@ type Delivery struct {
Plan *DeliveryPlan `json:"plan,omitempty"`
// Check is its own check's verdict.
Check *Verdict `json:"check,omitempty"`
// CheckAsk is this owner's own ask of its check, when the forge's announcement did not bring one: a head
// adopted while it already waited, or one whose announcement the controller missed.
CheckAsk *CheckAsk `json:"check_ask,omitempty"`
// The facts the table's guards read. NewerHead is the head that superseded it; ClosedUnmerged the pull
// request closed; MergedAs the commit it landed on the trunk as; HeldWhy what holds it for a person.
@@ -87,6 +90,17 @@ type Verdict struct {
RepoSaid string `json:"repo_summary,omitempty"`
}
// CheckAsk is a head's own check, asked of the controller by this owner (or re-asked by healer H2).
type CheckAsk struct {
// ID is the ask's id as the controller answered it.
ID string `json:"id"`
At time.Time `json:"at"`
By string `json:"by"`
Why string `json:"why"`
// Reasked is how many times H2 asked it again: once at most, then the operator's.
Reasked int `json:"reasked,omitempty"`
}
// Passes is whether the verdict lets the delivery be ready: the gate passed or warned, and the
// repository's own check did not fail or error.
func (v *Verdict) Passes() bool {
@@ -1,6 +1,7 @@
package main
import (
"errors"
"fmt"
"strings"
"time"
@@ -53,6 +54,7 @@ const (
EvAccepted Event = "accepted" // the verdict passes
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
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
@@ -138,6 +140,22 @@ var Table = []Row{
{From: []State{Rejected, Ready}, Event: EvRecheck, To: Proposed, Act: true,
Guard: "a person asked, with why, or its group changed",
Holds: func(d *Delivery, f Facts) error { return unless(f.Why != "", "a recheck says why") }},
{From: []State{Proposed}, Event: EvReask, To: Proposed, Act: true,
Guard: "its check was asked and no verdict came within its bound, and it was not re-asked before: healer H2 " +
"asks it once more, with why — after that it is the operator's",
Holds: func(d *Delivery, f Facts) error {
switch {
case f.Why == "" || f.By == "":
return errors.New("a re-ask says who and why")
case d.Check != nil:
return errors.New("its verdict is known")
case d.CheckAsk == nil:
return errors.New("its check was never asked: the tick asks it, not H2")
case d.CheckAsk.Reasked > 0:
return errors.New("its check was re-asked once already: the operator's")
}
return nil
}},
{From: []State{Published}, Event: EvSuperseded, To: Superseded,
Guard: "a newer delivery to the same trunk took over its walk",
@@ -301,13 +319,38 @@ type Bound struct {
// Bounds are every state's.
var Bounds = map[State]Bound{
Proposed: {For: time.Hour, Says: "the check's own watchdog (S6) speaks for a check that is late"},
Proposed: {For: time.Hour, H2: []Event{EvReask}, Says: "its check was asked and not answered: H2 asks it once " +
"more, then the check's own watchdog (S6) and the operator speak for it"},
Checked: {For: time.Minute, Says: "a verdict is decided at once"},
Published: {For: 30 * time.Minute, H2: []Event{EvSuperseded, EvGo}, Says: "its walk waits for its turn or its word"},
Held: {For: 24 * time.Hour, Says: "it waits for the operator"},
Delivering: {For: 2 * time.Hour, H2: []Event{EvDone, EvSuperseded, EvFailed}, Says: "its walk runs"},
}
// checkGrace is how long a proposed delivery is left to the controller's own check of its announcement before
// this owner asks the check itself: longer than a check takes from announcement to verdict. A head adopted while
// it already waited — announced again from the bus's history — is asked once it has waited this long too.
const checkGrace = 15 * time.Minute
// boundSince is when a delivery's bound runs from: its state's start, except for a proposed delivery whose
// check this owner asked — its bound is "asked and not answered", so it runs from the ask. One never asked
// runs from its start plus the grace before it is asked: an ask that cannot be made is still listed.
func boundSince(d *Delivery) time.Time {
if d.State == Proposed {
if d.CheckAsk != nil {
return d.CheckAsk.At
}
return d.Since.Add(checkGrace)
}
return d.Since
}
// pastBound is whether a delivery has been where it is longer than its state's bound.
func pastBound(d *Delivery, now time.Time) bool {
b, bounded := Bounds[d.State]
return bounded && !d.State.Final() && now.Sub(boundSince(d)) > b.For
}
// StepState is where one machine stands in a delivering walk.
type StepState string
@@ -41,6 +41,9 @@ func aDeliveryFor(r Row) (*Delivery, Facts) {
f = Facts{Now: now}
d.Walk = &WalkSeen{ID: "plan-1", State: walkFailed, StoppedBy: "mesh-delivery for jochen"}
}
case EvReask:
f.Why, f.By = "delivery stalled", byH2
d.CheckAsk = &CheckAsk{ID: "check-1", At: now.Add(-2 * time.Hour), By: byOwner}
case EvHold:
d.HeldWhy, d.MergedAs = "its group's check did not pass", "aaaa1111"
case EvDone:
@@ -91,7 +94,7 @@ func TestEveryRowOfTheTableIsTakenWhenItsGuardHoldsAndRefusedWhenNot(t *testing.
}
func TestEveryPairTheTableDoesNotHoldIsRefusedByName(t *testing.T) {
events := []Event{EvAnnounced, EvAppeared, EvAdopted, EvChecked, EvAccepted, EvRefused, EvRecheck, EvNewHead,
events := []Event{EvAnnounced, EvAppeared, EvAdopted, EvChecked, EvAccepted, EvRefused, EvRecheck, EvReask, EvNewHead,
EvClosed, EvMerged, EvMergedUnchecked, EvGo, EvHold, EvRelease, EvDone, EvFailed, EvSuperseded, EvStop}
for _, from := range append([]State{None}, AllStates...) {
for _, ev := range events {
@@ -145,14 +148,19 @@ func TestTheTableKeepsItsRules(t *testing.T) {
}
}
// Every state that is not final has a bound or is the operator's to wait in, and every H2 transition is
// a row the table holds from that state.
// a row the table holds from that state: observed once the walk's record is read again, or H2's own ask.
for state, b := range Bounds {
for _, ev := range b.H2 {
if rowFor(state, ev, false) == nil {
if rowFor(state, ev, false) == nil && rowFor(state, ev, true) == nil {
t.Errorf("H2 may take %s from %s, which the table does not hold", ev, state)
}
}
}
// H2 re-asks a check once: a second re-ask is refused by the table itself.
d0 := &Delivery{ID: "novox/app@c0", State: Proposed, Number: 1, CheckAsk: &CheckAsk{ID: "check-1", Reasked: 1}}
if _, err := Apply(d0, EvReask, true, Facts{Now: time.Now(), By: byH2, Why: "stalled"}); err == nil {
t.Fatal("a check re-asked once was re-asked again")
}
// A ready delivery off the trunk is refused publication, whatever else is true of it.
d := &Delivery{ID: "novox/app@c0", State: Ready, Walk: &WalkSeen{ID: "plan-x", State: walkRolling}}
if _, err := Apply(d, EvMerged, false, Facts{Now: time.Now()}); err == nil {
@@ -73,9 +73,7 @@ func (h *Holder) Deliveries(state, repository, group string, all bool) []Line {
if d.Walk != nil {
l.Walk = d.Walk.ID
}
if b, bounded := Bounds[d.State]; bounded && !d.State.Final() && now.Sub(d.Since) > b.For {
l.Stalled = true
}
l.Stalled = pastBound(d, now)
out = append(out, l)
}
sort.Slice(out, func(i, j int) bool {
@@ -166,19 +164,26 @@ func (h *Holder) Stalled() []StalledLine {
now := h.Now()
var out []StalledLine
for _, d := range h.deliveries {
b, bounded := Bounds[d.State]
if !bounded || d.State.Final() || now.Sub(d.Since) <= b.For {
if !pastBound(d, now) {
continue
}
b := Bounds[d.State]
h2 := "none: the state is the operator's"
if len(b.H2) > 0 {
switch {
case d.State == Proposed && d.CheckAsk == nil:
h2 = "none: its check could not be asked yet — the controller does not answer delivery-check"
case d.State == Proposed && d.CheckAsk.Reasked > 0:
h2 = "none: its check was re-asked once already and still not answered — the operator's"
case d.State == Proposed:
h2 = "close, by re-ask: its check asked once more"
case len(b.H2) > 0:
var evs []string
for _, e := range b.H2 {
evs = append(evs, string(e))
}
h2 = "close, by " + strings.Join(evs, " or ") + ", when the walk's record says so"
}
out = append(out, StalledLine{ID: d.ID, State: d.State, For: now.Sub(d.Since).Round(time.Second).String(),
out = append(out, StalledLine{ID: d.ID, State: d.State, For: now.Sub(boundSince(d)).Round(time.Second).String(),
Bound: b.For.String(), H2: h2, Says: b.Says})
}
sort.Slice(out, func(i, j int) bool { return out[i].ID < out[j].ID })
@@ -302,10 +307,13 @@ func (h *Holder) Close(id, why string) (string, error) {
h.mu.Unlock()
return "", fmt.Errorf("%s is %s: H2 has no transition there — the state is the operator's", id, d.State)
}
if h.Now().Sub(d.Since) <= b.For {
if !pastBound(d, h.Now()) {
h.mu.Unlock()
return "", fmt.Errorf("%s has been %s for %s, within its bound of %s: nothing to close", id, d.State,
h.Now().Sub(d.Since).Round(time.Second), b.For)
h.Now().Sub(boundSince(d)).Round(time.Second), b.For)
}
if d.State == Proposed {
return h.reask(d, why) // unlocks
}
if d.Walk == nil {
h.mu.Unlock()
@@ -329,6 +337,40 @@ func (h *Holder) Close(id, why string) (string, error) {
return fmt.Sprintf("%s: %s → %s, as its walk's record says (%s)", id, was, d.State, why), nil
}
// reask is H2's close of a proposed delivery past its bound: its check, asked and not answered, asked once
// more — the table's re-ask, which holds only once. The row is tried first on a copy, so an ask the controller
// does not take spends nothing. Called under the lock; it unlocks.
func (h *Holder) reask(d *Delivery, why string) (string, error) {
id := d.ID
trial := *d
trial.Transitions = nil
if _, err := Apply(&trial, EvReask, true, Facts{Now: h.Now(), By: byH2, Why: why}); err != nil {
h.mu.Unlock()
return "", err
}
m := MemberOf(d)
h.mu.Unlock()
asked, err := h.Controller.Check("", []Member{m})
if err != nil {
return "", fmt.Errorf("%s's check could not be re-asked yet: %w", id, err)
}
h.mu.Lock()
defer h.mu.Unlock()
d = h.deliveries[id]
if d == nil {
return "", fmt.Errorf("no delivery %q", id)
}
t, err := Apply(d, EvReask, true, Facts{Now: h.Now(), By: byH2, Why: why + " — re-asked as " + asked})
if err != nil {
return "", err
}
d.CheckAsk.Reasked++
d.CheckAsk.ID, d.CheckAsk.At, d.CheckAsk.By, d.CheckAsk.Why = asked, h.Now(), byH2, why
h.moved(d, []Transition{t})
return fmt.Sprintf("%s: its check re-asked as %s, once (%s); if that is not answered either, the operator's", id,
asked, why), nil
}
// WhatIf is the `what-if` verb.
func (h *Holder) WhatIf(repository, base string, paths []string) (*DeliveryPlan, error) {
if base == "" {