diff --git a/modules/gitea/README.md b/modules/gitea/README.md index 08c51f0f..557e673f 100644 --- a/modules/gitea/README.md +++ b/modules/gitea/README.md @@ -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..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. diff --git a/modules/mesh-delivery/cmd/mesh-delivery/issue438_test.go b/modules/mesh-delivery/cmd/mesh-delivery/issue438_test.go index 7b78e952..6467cbeb 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/issue438_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/issue438_test.go @@ -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) { diff --git a/modules/mesh-delivery/cmd/mesh-delivery/unchecked.go b/modules/mesh-delivery/cmd/mesh-delivery/unchecked.go index 9f1ea3ba..9bef2f7d 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/unchecked.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/unchecked.go @@ -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}) }