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
|
`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})
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user