mesh-delivery: a check that answers keeps the hour running, and a started head is unanswered (issue 438)
mesh/merge-gate pass: builds gitea, mesh-delivery → novox; no bus step; every machine composes with the change as it did without (4 of 4 compose)
mesh/repo-check pass: its merge-check.sh passed, each part in its toolchain: merge-check.sh (go), merge-check-typescript.sh (typescript)
mesh/delivery superseded: a newer head of the same pull request

Review of #195: the clock restarted on any change in what a head waits on, so a
check answering pushed the line back by the time it took; it now restarts only on
a newer ask. A head whose check started was announced, so its line no longer says
unannounced; pending times are given in UTC like the module's other lines.
This commit is contained in:
2026-10-11 02:22:16 +02:00
parent 9d766b675d
commit 1f48a29495
3 changed files with 86 additions and 21 deletions
+3 -2
View File
@@ -19,8 +19,9 @@ Every new head of an open pull request is announced as `pull.updated`, once, and
`mesh/repo-check` with neither for ten minutes since it first saw it so is a delivery held past its bound `mesh/repo-check` with neither for ten minutes since it first saw it so is a delivery held past its bound
(`delivery.<id>.stalled`, state `unannounced`), which the controller raises like any other (issue 347). (`delivery.<id>.stalled`, state `unannounced`), which the controller raises like any other (issue 347).
Each of the head's statuses comes with its state and when it was set, so a head whose check started and never Each of the head's statuses comes with its state and when it was set, so a head whose check started and never
answered — a required check pending, or never set beside one that is — is said the same way once that has lasted answered — a required check pending, or never set beside one that is — is said as a stalled line in the state
the delivery table's bound for a check asked and not answered, an hour (issue 438). `unanswered` once that has lasted the delivery table's bound for a check asked and not answered, an hour, counted
from its first sight and again only when a check is asked anew (issue 438).
- **How to see it work.** A pull request opened more than a minute after its branch was pushed — this fix's own - **How to see it work.** A pull request opened more than a minute after its branch was pushed — this fix's own
was one, and the announcer it fixes never saw it — has `mesh/merge-gate` pending within a pass of the next sweep was one, and the announcer it fixes never saw it — has `mesh/merge-gate` pending within a pass of the next sweep
at the latest, and `mesh-controller.conditions` names none for it. at the latest, and `mesh-controller.conditions` names none for it.
@@ -56,7 +56,7 @@ func TestAHeadWhoseCheckStartedAndNeverAnsweredIsSaid(t *testing.T) {
} }
l := lines[0] l := lines[0]
if l.ID != IDOf("novox/mesh-controller", "c11222026a3b26a0cc4b277119e1ea059b82a36b") || l.Number != 212 || if l.ID != IDOf("novox/mesh-controller", "c11222026a3b26a0cc4b277119e1ea059b82a36b") || l.Number != 212 ||
l.State != Unannounced || l.Bound != Bounds[Proposed].For.String() || l.For != "1h1m0s" { l.State != Unanswered || l.Bound != Bounds[Proposed].For.String() || l.For != "1h1m0s" {
t.Fatalf("the line is not this head's, past the table's bound: %+v", l) t.Fatalf("the line is not this head's, past the table's bound: %+v", l)
} }
for _, want := range []string{"mesh/merge-gate", "pending since 2026-10-11T00:11:39Z", "mesh/repo-check", "never set", for _, want := range []string{"mesh/merge-gate", "pending since 2026-10-11T00:11:39Z", "mesh/repo-check", "never set",
@@ -73,12 +73,42 @@ func TestAHeadWhoseCheckStartedAndNeverAnsweredIsSaid(t *testing.T) {
w.h.PullUpdated(pr("novox/mesh-controller", 212, "c11222026a3b26a0cc4b277119e1ea059b82a36b", "fix/402")) w.h.PullUpdated(pr("novox/mesh-controller", 212, "c11222026a3b26a0cc4b277119e1ea059b82a36b", "fix/402"))
w.h.SeeOpenPulls() w.h.SeeOpenPulls()
for _, l := range w.h.Stalled() { for _, l := range w.h.Stalled() {
if l.State == Unannounced { if l.State == Unannounced || l.State == Unanswered {
t.Fatalf("an announced head is still said unannounced: %+v", l) t.Fatalf("an announced head is still said unannounced: %+v", l)
} }
} }
} }
// **A check that answers does not start the bound again**: merge-gate answers at fifty minutes, repo-check is never
// set, and the head is said on the bound from this owner's first sight of it, not fifty minutes later.
func TestACheckThatAnswersDoesNotStartTheBoundAgain(t *testing.T) {
w := newWorldOver(t, newMemStore(), newFakeController(), newFakeForge(), time.Date(2026, 10, 11, 0, 12, 5, 0, time.UTC))
w.forge.open = openPullsOf(t, startedNeverAnswered)
w.h.SeeOpenPulls()
w.later(50 * time.Minute)
w.forge.open = openPullsOf(t, strings.Replace(startedNeverAnswered, `"state": "pending", "description": "the mesh is mapping`,
`"state": "success", "description": "pass: the mesh is mapping`, 1))
w.h.SeeOpenPulls()
w.later(11 * time.Minute)
w.h.SeeOpenPulls()
lines := w.h.Stalled()
if len(lines) != 1 || lines[0].For != "1h1m0s" || !strings.Contains(lines[0].Says, "mesh/repo-check never set") ||
strings.Contains(lines[0].Says, "mesh/merge-gate") {
t.Fatalf("a head whose merge-gate answered at fifty minutes is not said at sixty-one on what is left: %+v", lines)
}
}
// A forge time with an offset is said in UTC, as the module's other lines say their times.
func TestAPendingTimeIsSaidInUTC(t *testing.T) {
w := newWorldOver(t, newMemStore(), newFakeController(), newFakeForge(), time.Date(2026, 10, 11, 0, 12, 5, 0, time.UTC))
w.forge.open = openPullsOf(t, strings.Replace(startedNeverAnswered, "2026-10-11T00:11:39Z", "2026-10-11T02:11:39+02:00", 1))
w.h.SeeOpenPulls()
w.later(Bounds[Proposed].For + time.Minute)
if lines := w.h.Stalled(); len(lines) != 1 || !strings.Contains(lines[0].Says, "pending since 2026-10-11T00:11:39Z") {
t.Fatalf("the pending time is not said in UTC: %+v", lines)
}
}
// A check asked again — its pending status set anew, as a recheck or a new announcement does — starts the bound again: // A check asked again — its pending status set anew, as a recheck or a new announcement does — starts the bound again:
// the head is said only once that ask too has gone unanswered past it. // the head is said only once that ask too has gone unanswered past it.
func TestACheckAskedAgainStartsItsBoundAgain(t *testing.T) { func TestACheckAskedAgainStartsItsBoundAgain(t *testing.T) {
@@ -24,12 +24,18 @@ import (
// required check pending or never set is said once that has lasted the delivery table's bound for a check asked and // required check pending or never set is said once that has lasted the delivery table's bound for a check asked and
// not answered (Bounds[Proposed]): an announced head would be said after that bound too, and it is longer than the // not answered (Bounds[Proposed]): an announced head would be said after that bound too, and it is longer than the
// controller lets one check run (its checkTimeout, 45 minutes), so a slow check or a status read a moment behind // controller lets one check run (its checkTimeout, 45 minutes), so a slow check or a status read a moment behind
// (issue 423) is never said. // (issue 423) is never said. Such a head was announced — the controller set its status — so its line is not in the
// state Unannounced but Unanswered, with words of its own.
// Unannounced is a pull request's head the forge never announced: not a state of the table — no delivery exists — // Unannounced is a pull request's head the forge never announced: not a state of the table — no delivery exists —
// only what a stalled line says. // only what a stalled line says.
const Unannounced State = "unannounced" const Unannounced State = "unannounced"
// Unanswered is a pull request's head whose merge check started and never answered, and no delivery of it is held
// here (novox/hq issue 438): not a state of the table either, only what a stalled line says. The word is the table's
// own for the proposed state's bound: its check was asked and not answered.
const Unanswered State = "unanswered"
// UncheckedAfter is how long a head may go without a merge check before it is said: the announcer passes every half // UncheckedAfter is how long a head may go without a merge check before it is said: the announcer passes every half
// minute and the check is asked at once, so ten minutes is a fault, never a queue. // minute and the check is asked at once, so ten minutes is a fault, never a queue.
const UncheckedAfter = 10 * time.Minute const UncheckedAfter = 10 * time.Minute
@@ -68,16 +74,20 @@ type HeadCheck struct {
type unchecked struct { type unchecked struct {
pull OpenPull pull OpenPull
waiting []string waiting []string
// pending is when each required check that is pending was set so, as the forge says it: what tells a check
// asked again from one still waiting on the same ask.
pending map[string]string
first time.Time first time.Time
} }
// unanswered are the merge checks the base requires that the head has no verdict of, each as it reads — "never // unanswered are the merge checks the base requires that the head has no verdict of, each as it reads — "never
// set", or pending and since when — and whether the head has any merge check at all: its check started. A head on // set", or pending and since when — when each pending one was set so, and whether the head has any merge check at
// a base that requires none, or without a head, has nothing unanswered. // all: its check started. A head on a base that requires none, or without a head, has nothing unanswered.
func (p OpenPull) unanswered() (waiting []string, started bool) { func (p OpenPull) unanswered() (waiting []string, pending map[string]string, started bool) {
if p.HeadSHA == "" { if p.HeadSHA == "" {
return nil, false return nil, nil, false
} }
pending = map[string]string{}
started = slices.ContainsFunc(mergeChecks, func(c string) bool { started = slices.ContainsFunc(mergeChecks, func(c string) bool {
return slices.Contains(p.Statuses, c) || p.Checks[c].State != "" return slices.Contains(p.Statuses, c) || p.Checks[c].State != ""
}) })
@@ -93,10 +103,20 @@ func (p OpenPull) unanswered() (waiting []string, started bool) {
case !ok || got.State == "": case !ok || got.State == "":
waiting = append(waiting, c+" never set") waiting = append(waiting, c+" never set")
case got.State == "pending": case got.State == "pending":
waiting = append(waiting, c+" pending since "+orUnknown(got.UpdatedAt)) pending[c] = utcOf(got.UpdatedAt)
waiting = append(waiting, c+" pending since "+orUnknown(pending[c]))
} }
} }
return waiting, started return waiting, pending, started
}
// utcOf is a forge time in UTC, as this module's other lines give their times; one it cannot read stays as given.
func utcOf(at string) string {
t, err := time.Parse(time.RFC3339, at)
if err != nil {
return at
}
return t.UTC().Format(time.RFC3339)
} }
func orUnknown(at string) string { func orUnknown(at string) string {
@@ -106,6 +126,18 @@ func orUnknown(at string) string {
return at return at
} }
// askedAgain is whether a required check is pending on a newer ask than when this owner last saw the head: set
// pending for the first time, set pending anew, or gone back to pending after it answered. A check that answers
// never is: the head waits on what is left from the same first sight (novox/hq issue 438).
func askedAgain(was, now map[string]string) bool {
for c, at := range now {
if before, ok := was[c]; !ok || before != at {
return true
}
}
return false
}
// SeeOpenPulls asks the forge for every open pull request and keeps the heads with a required merge check not // SeeOpenPulls asks the forge for every open pull request and keeps the heads with a required merge check not
// answered. A forge that cannot be asked changes nothing: what was seen stands, and nothing is guessed. // answered. A forge that cannot be asked changes nothing: what was seen stands, and nothing is guessed.
func (h *Holder) SeeOpenPulls() { func (h *Holder) SeeOpenPulls() {
@@ -119,7 +151,7 @@ func (h *Holder) SeeOpenPulls() {
now := h.Now() now := h.Now()
seen := map[string]unchecked{} seen := map[string]unchecked{}
for _, p := range open { for _, p := range open {
waiting, _ := p.unanswered() waiting, pending, _ := p.unanswered()
if len(waiting) == 0 { if len(waiting) == 0 {
continue continue
} }
@@ -127,24 +159,24 @@ func (h *Holder) SeeOpenPulls() {
if h.deliveries[id] != nil { if h.deliveries[id] != nil {
continue // announced: a delivery with its own bound continue // announced: a delivery with its own bound
} }
// The clock runs from when this owner first saw the head waiting as it waits now: a check that started, or // The clock runs from when this owner first saw the head waiting, and starts again only on a newer ask: a
// was asked again — its pending status set anew — is given its whole bound from then. // check that started, or was asked again — its pending status set anew — is given its whole bound from then.
first := now // A check that answers does not start it again, so a head is said on the bound from its first sight.
if was, ok := h.unchecked[id]; ok && slices.Equal(was.waiting, waiting) { u := unchecked{pull: p, waiting: waiting, pending: pending, first: now}
first = was.first if was, ok := h.unchecked[id]; ok && !askedAgain(was.pending, pending) {
u.first = was.first
} else { } else {
u := unchecked{pull: p, waiting: waiting}
h.Logf("[mesh-delivery] %s/%s#%d's head %s %s; said if so in %s", p.Owner, p.Repo, p.Number, h.Logf("[mesh-delivery] %s/%s#%d's head %s %s; said if so in %s", p.Owner, p.Repo, p.Number,
short(p.HeadSHA), u.what(), u.bound()) short(p.HeadSHA), u.what(), u.bound())
} }
seen[id] = unchecked{pull: p, waiting: waiting, first: first} seen[id] = u
} }
h.unchecked = seen h.unchecked = seen
} }
// started is whether the head has any merge check: its check started, rather than was never asked. // started is whether the head has any merge check: its check started, rather than was never asked.
func (u unchecked) started() bool { func (u unchecked) started() bool {
_, started := u.pull.unanswered() _, _, started := u.pull.unanswered()
return started return started
} }
@@ -176,14 +208,16 @@ func (h *Holder) uncheckedLines(now time.Time) []StalledLine {
says := fmt.Sprintf("%s/%s#%d is open on %s, which requires the merge check, and its head has had no merge "+ says := fmt.Sprintf("%s/%s#%d is open on %s, which requires the merge check, and its head has had no merge "+
"check: the mesh was never asked to check it. A new commit on its branch announces it", "check: the mesh was never asked to check it. A new commit on its branch announces it",
u.pull.Owner, u.pull.Repo, u.pull.Number, u.pull.Base) u.pull.Owner, u.pull.Repo, u.pull.Number, u.pull.Base)
state := Unannounced
if u.started() { if u.started() {
state = Unanswered
says = fmt.Sprintf("%s/%s#%d is open on %s, which requires the merge check, and its head's check started "+ says = fmt.Sprintf("%s/%s#%d is open on %s, which requires the merge check, and its head's check started "+
"and never answered: %s. No delivery of it is held here, so nothing asks it again. A new commit on "+ "and never answered: %s. No delivery of it is held here, so nothing asks it again. A new commit on "+
"its branch announces it and asks its check", u.pull.Owner, u.pull.Repo, u.pull.Number, u.pull.Base, "its branch announces it and asks its check", u.pull.Owner, u.pull.Repo, u.pull.Number, u.pull.Base,
strings.Join(u.waiting, ", ")) strings.Join(u.waiting, ", "))
h2 = "none: no delivery of it is held here, so there is none to close — the operator's" h2 = "none: no delivery of it is held here, so there is none to close — the operator's"
} }
out = append(out, StalledLine{ID: id, Number: u.pull.Number, State: Unannounced, For: now.Sub(u.first).Round(time.Second).String(), out = append(out, StalledLine{ID: id, Number: u.pull.Number, State: state, For: now.Sub(u.first).Round(time.Second).String(),
Bound: u.bound().String(), Bound: u.bound().String(),
H2: h2, Says: says}) H2: h2, Says: says})
} }