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
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:
@@ -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
|
||||
(`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
|
||||
answered — a required check pending, or never set beside one that is — is said the same way once that has lasted
|
||||
the delivery table's bound for a check asked and not answered, an hour (issue 438).
|
||||
answered — a required check pending, or never set beside one that is — is said as a stalled line in the state
|
||||
`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
|
||||
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.
|
||||
|
||||
@@ -56,7 +56,7 @@ func TestAHeadWhoseCheckStartedAndNeverAnsweredIsSaid(t *testing.T) {
|
||||
}
|
||||
l := lines[0]
|
||||
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)
|
||||
}
|
||||
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.SeeOpenPulls()
|
||||
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)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// **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:
|
||||
// the head is said only once that ask too has gone unanswered past it.
|
||||
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
|
||||
// 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
|
||||
// (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 —
|
||||
// only what a stalled line says.
|
||||
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
|
||||
// minute and the check is asked at once, so ten minutes is a fault, never a queue.
|
||||
const UncheckedAfter = 10 * time.Minute
|
||||
@@ -68,16 +74,20 @@ type HeadCheck struct {
|
||||
type unchecked struct {
|
||||
pull OpenPull
|
||||
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
|
||||
}
|
||||
|
||||
// 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
|
||||
// a base that requires none, or without a head, has nothing unanswered.
|
||||
func (p OpenPull) unanswered() (waiting []string, started bool) {
|
||||
// set", or pending and since when — when each pending one was set so, and whether the head has any merge check at
|
||||
// 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, pending map[string]string, started bool) {
|
||||
if p.HeadSHA == "" {
|
||||
return nil, false
|
||||
return nil, nil, false
|
||||
}
|
||||
pending = map[string]string{}
|
||||
started = slices.ContainsFunc(mergeChecks, func(c string) bool {
|
||||
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 == "":
|
||||
waiting = append(waiting, c+" never set")
|
||||
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 {
|
||||
@@ -106,6 +126,18 @@ func orUnknown(at string) string {
|
||||
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
|
||||
// answered. A forge that cannot be asked changes nothing: what was seen stands, and nothing is guessed.
|
||||
func (h *Holder) SeeOpenPulls() {
|
||||
@@ -119,7 +151,7 @@ func (h *Holder) SeeOpenPulls() {
|
||||
now := h.Now()
|
||||
seen := map[string]unchecked{}
|
||||
for _, p := range open {
|
||||
waiting, _ := p.unanswered()
|
||||
waiting, pending, _ := p.unanswered()
|
||||
if len(waiting) == 0 {
|
||||
continue
|
||||
}
|
||||
@@ -127,24 +159,24 @@ func (h *Holder) SeeOpenPulls() {
|
||||
if h.deliveries[id] != nil {
|
||||
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
|
||||
// was asked again — its pending status set anew — is given its whole bound from then.
|
||||
first := now
|
||||
if was, ok := h.unchecked[id]; ok && slices.Equal(was.waiting, waiting) {
|
||||
first = was.first
|
||||
// The clock runs from when this owner first saw the head waiting, and starts again only on a newer ask: a
|
||||
// check that started, or was asked again — its pending status set anew — is given its whole bound from then.
|
||||
// A check that answers does not start it again, so a head is said on the bound from its first sight.
|
||||
u := unchecked{pull: p, waiting: waiting, pending: pending, first: now}
|
||||
if was, ok := h.unchecked[id]; ok && !askedAgain(was.pending, pending) {
|
||||
u.first = was.first
|
||||
} 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,
|
||||
short(p.HeadSHA), u.what(), u.bound())
|
||||
}
|
||||
seen[id] = unchecked{pull: p, waiting: waiting, first: first}
|
||||
seen[id] = u
|
||||
}
|
||||
h.unchecked = seen
|
||||
}
|
||||
|
||||
// started is whether the head has any merge check: its check started, rather than was never asked.
|
||||
func (u unchecked) started() bool {
|
||||
_, started := u.pull.unanswered()
|
||||
_, _, started := u.pull.unanswered()
|
||||
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 "+
|
||||
"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)
|
||||
state := Unannounced
|
||||
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 "+
|
||||
"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,
|
||||
strings.Join(u.waiting, ", "))
|
||||
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(),
|
||||
H2: h2, Says: says})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user