novox/mesh-catalog
6
0
Fork 0
Code Issues Pull Requests 4 Actions Packages Projects Releases Wiki Activity

Merge pull request 'mesh-delivery: say a ready or rejected delivery whose check has not answered on the forge (hq issue 441)' (#199) from fix/441-a-held-delivery-without-its-check-is-said into main

Browse Source
This commit was merged in pull request #199.
This commit is contained in:
mesh-admin
2026-10-11 01:26:37 +00:00
parent ebd940fcd0 9bd8903ed2
commit a5b4a757ec
2 changed files with 290 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files
modules/mesh-delivery/cmd/mesh-delivery/issue441_test.go
+251
View File
@@ -0,0 +1,251 @@
package main
import (
"slices"
"strings"
"testing"
"time"
)
// A delivery held in ready or rejected whose required merge check reads pending or missing on the forge
// (novox/hq issue 441). The pull requests below are mesh-controller #215's head, ready, and mesh-lab #76's, rejected,
// as gitea_open_pulls said them on 2026-10-11, with only the one status changed that the case is about: a status
// write that lagged or was lost (issue 423), or a check asked again after the verdict.
const readyHeadSHA = "d2e9dc61590f950b74ae2e93282c354646705188"
// readyRepoCheckPending is #215 with mesh/repo-check pending since its delivery was ready.
const readyRepoCheckPending = `{"pulls": [{
"base": "main", "head_sha": "d2e9dc61590f950b74ae2e93282c354646705188",
"html_url": "http://127.0.0.1:20000/novox/mesh-controller/pulls/215", "number": 215,
"owner": "novox", "repo": "mesh-controller",
"required": ["mesh/merge-gate", "mesh/repo-check"],
"statuses": ["mesh/merge-gate", "mesh/repo-check", "mesh/delivery"],
"checks": {
"mesh/delivery": {"state": "success", "description": "ready: it delivers once merged", "updated_at": "2026-10-11T00:42:06Z"},
"mesh/merge-gate": {"state": "success", "description": "pass: builds build-agent → ace, g14, novox, shanks; no bus step; every machine composes with the change as it did without (4 of 4 compose)", "updated_at": "2026-10-11T00:42:01Z"},
"mesh/repo-check": {"state": "pending", "description": "the repository's own check runs", "updated_at": "2026-10-11T00:42:01Z"}
}
}]}`
// readyRepoCheckMissing is #215 whose mesh/repo-check was never written.
const readyRepoCheckMissing = `{"pulls": [{
"base": "main", "head_sha": "d2e9dc61590f950b74ae2e93282c354646705188", "number": 215,
"owner": "novox", "repo": "mesh-controller",
"required": ["mesh/merge-gate", "mesh/repo-check"],
"statuses": ["mesh/merge-gate", "mesh/delivery"],
"checks": {
"mesh/delivery": {"state": "success", "description": "ready: it delivers once merged", "updated_at": "2026-10-11T00:42:06Z"},
"mesh/merge-gate": {"state": "success", "description": "pass", "updated_at": "2026-10-11T00:42:01Z"}
}
}]}`
// readyAnswered is #215 as the forge said it: every required check answered.
const readyAnswered = `{"pulls": [{
"base": "main", "head_sha": "d2e9dc61590f950b74ae2e93282c354646705188", "number": 215,
"owner": "novox", "repo": "mesh-controller",
"required": ["mesh/merge-gate", "mesh/repo-check"],
"statuses": ["mesh/merge-gate", "mesh/repo-check", "mesh/delivery"],
"checks": {
"mesh/merge-gate": {"state": "success", "description": "pass", "updated_at": "2026-10-11T00:42:01Z"},
"mesh/repo-check": {"state": "success", "description": "pass: its merge-check.sh passed", "updated_at": "2026-10-11T00:42:01Z"}
}
}]}`
// heldIn is a delivery of #215's head in ready (a passing verdict) or rejected (a failing one).
func heldIn(t *testing.T, w *world, gate string, want State) string {
t.Helper()
w.h.PullUpdated(pr("novox/mesh-controller", 215, readyHeadSHA, "fix/215"))
w.h.Checked(verdict("novox/mesh-controller", 215, readyHeadSHA, gate))
id := IDOf("novox/mesh-controller", readyHeadSHA)
if s := w.state(id); s != want {
t.Fatalf("a %s verdict left it %s", gate, s)
}
return id
}
// **A delivery in ready whose required check reads pending on the forge is said** (novox/hq issue 441) on the same
// rule as a head with no delivery (issue 438): past the table's bound for a check asked and not answered, since this
// owner first saw it so, as an unanswered line naming the check and since when. Before, any head with a delivery was
// skipped, and ready has no bound of its own, so it waited for ever — unable to merge — with nothing said.
func TestADeliveryInReadyWhoseCheckIsPendingIsSaid(t *testing.T) {
w := newWorldOver(t, newMemStore(), newFakeController(), newFakeForge(), time.Date(2026, 10, 11, 0, 43, 0, 0, time.UTC))
id := heldIn(t, w, "pass", Ready)
w.forge.open = openPullsOf(t, readyRepoCheckPending)
w.h.SeeOpenPulls()
w.later(Bounds[Proposed].For - time.Minute)
w.h.SeeOpenPulls()
if lines := w.h.Stalled(); len(lines) != 0 {
t.Fatalf("a ready delivery still within the bound is said: %+v", lines)
}
w.later(2 * time.Minute)
lines := w.h.Stalled()
if len(lines) != 1 {
t.Fatalf("a ready delivery whose required check is pending past the bound is not said: %+v", lines)
}
l := lines[0]
if l.ID != id || l.Number != 215 || l.State != Unanswered || l.Bound != Bounds[Proposed].For.String() || l.For != "1h1m0s" {
t.Fatalf("the line is not this delivery's, past the table's bound: %+v", l)
}
if want := []string{"mesh/repo-check pending since 2026-10-11T00:42:01Z"}; !slices.Equal(l.Waiting, want) {
t.Fatalf("the line's checks waited on are %q, not %q", l.Waiting, want)
}
for _, want := range []string{"novox/mesh-controller#215", "ready", "mesh/repo-check pending since 2026-10-11T00:42:01Z", "recheck"} {
if !strings.Contains(l.Says, want) {
t.Fatalf("the line does not say %q: %s", want, l.Says)
}
}
if strings.Contains(l.Says, "No delivery of it is held") {
t.Fatalf("the line says no delivery is held of a head whose delivery is ready: %s", l.Says)
}
// The forge's status catches up: the check answered, and the line is gone.
w.forge.open = openPullsOf(t, readyAnswered)
w.h.SeeOpenPulls()
if lines := w.h.Stalled(); len(lines) != 0 {
t.Fatalf("a ready delivery whose check answered is still said: %+v", lines)
}
}
// **A delivery in rejected whose required check was never set is said** too: a lost status write leaves the pull
// request with no verdict to show, and nothing here asks again. It was announced — a delivery exists — so it is
// unanswered, never unannounced, though its head has no merge check at all.
func TestADeliveryInRejectedWhoseCheckIsMissingIsSaid(t *testing.T) {
w := newWorldOver(t, newMemStore(), newFakeController(), newFakeForge(), time.Date(2026, 10, 11, 0, 43, 0, 0, time.UTC))
id := heldIn(t, w, "fail", Rejected)
w.forge.open = openPullsOf(t, strings.Replace(readyRepoCheckMissing,
`"mesh/merge-gate": {"state": "success", "description": "pass", "updated_at": "2026-10-11T00:42:01Z"}`,
`"mesh/merge-gate": {"state": "failure", "description": "fail", "updated_at": "2026-10-11T00:42:01Z"}`, 1))
w.h.SeeOpenPulls()
w.later(Bounds[Proposed].For + time.Minute)
w.h.SeeOpenPulls()
lines := w.h.Stalled()
if len(lines) != 1 || lines[0].ID != id || lines[0].State != Unanswered ||
!slices.Equal(lines[0].Waiting, []string{"mesh/repo-check never set"}) || !strings.Contains(lines[0].Says, "rejected") {
t.Fatalf("a rejected delivery whose required check was never set is not said: %+v", lines)
}
// No merge check on its head at all — both writes lost — is still a delivery's: unanswered, never unannounced.
w2 := newWorldOver(t, newMemStore(), newFakeController(), newFakeForge(), time.Date(2026, 10, 11, 0, 43, 0, 0, time.UTC))
heldIn(t, w2, "fail", Rejected)
w2.forge.open = openPullsOf(t, `{"pulls": [{"base": "main", "head_sha": "`+readyHeadSHA+`", "number": 215,
"owner": "novox", "repo": "mesh-controller", "required": ["mesh/merge-gate", "mesh/repo-check"],
"statuses": ["mesh/delivery"], "checks": {"mesh/delivery": {"state": "failure", "description": "rejected"}}}]}`)
w2.h.SeeOpenPulls()
if lines := w2.h.Stalled(); len(lines) != 0 {
t.Fatalf("a delivery's head without a check is said at once, before the table's bound: %+v", lines)
}
w2.later(Bounds[Proposed].For + time.Minute)
w2.h.SeeOpenPulls()
lines = w2.h.Stalled()
if len(lines) != 1 || lines[0].State != Unanswered ||
!slices.Equal(lines[0].Waiting, []string{"mesh/merge-gate never set", "mesh/repo-check never set"}) {
t.Fatalf("a rejected delivery whose head has no merge check is not said as unanswered: %+v", lines)
}
}
// **A delivery in a state with a bound of its own is not said here**: a proposed one waits on its check under
// Bounds[Proposed] and H2's re-ask, and a second line for it would say the same thing twice.
func TestAProposedDeliveryIsSaidByItsOwnBoundOnly(t *testing.T) {
w := newWorldOver(t, newMemStore(), newFakeController(), newFakeForge(), time.Date(2026, 10, 11, 0, 43, 0, 0, time.UTC))
w.h.PullUpdated(pr("novox/mesh-controller", 215, readyHeadSHA, "fix/215"))
id := IDOf("novox/mesh-controller", readyHeadSHA)
w.forge.open = openPullsOf(t, readyRepoCheckPending)
w.h.SeeOpenPulls()
w.later(3 * Bounds[Proposed].For)
w.h.SeeOpenPulls()
for _, l := range w.h.Stalled() {
if l.State == Unanswered || l.State == Unannounced {
t.Fatalf("a proposed delivery %s is said as a head without a delivery: %+v", id, l)
}
}
}
// **A ready delivery whose required checks answered is not said**, as the forge said every one on 2026-10-11.
func TestAReadyDeliveryWhoseChecksAnsweredIsNotSaid(t *testing.T) {
w := newWorldOver(t, newMemStore(), newFakeController(), newFakeForge(), time.Date(2026, 10, 11, 0, 43, 0, 0, time.UTC))
heldIn(t, w, "pass", Ready)
w.forge.open = openPullsOf(t, readyAnswered)
w.h.SeeOpenPulls()
w.later(3 * Bounds[Proposed].For)
w.h.SeeOpenPulls()
if lines := w.h.Stalled(); len(lines) != 0 {
t.Fatalf("a ready delivery whose checks answered is said: %+v", lines)
}
}
// saidReady is a ready delivery of #215's head whose mesh/repo-check reads pending on the forge, said past the bound.
func saidReady(t *testing.T) (*world, string) {
t.Helper()
w := newWorldOver(t, newMemStore(), newFakeController(), newFakeForge(), time.Date(2026, 10, 11, 0, 43, 0, 0, time.UTC))
id := heldIn(t, w, "pass", Ready)
w.forge.open = openPullsOf(t, readyRepoCheckPending)
w.h.SeeOpenPulls()
w.later(Bounds[Proposed].For + time.Minute)
if lines := w.h.Stalled(); len(lines) != 1 || lines[0].ID != id || lines[0].State != Unanswered {
t.Fatalf("a ready delivery whose check is pending past the bound is not said: %+v", lines)
}
return w, id
}
// unansweredOf are the unanswered lines among what `stalled` says.
func unansweredOf(w *world) []StalledLine {
var out []StalledLine
for _, l := range w.h.Stalled() {
if l.State == Unanswered {
out = append(out, l)
}
}
return out
}
// **A said line clears when its delivery's state changes** (novox/hq issue 441): a recheck proposes it again, under
// the proposed state's own bound, and a merge publishes it, past the pull request. Both clear the line at once —
// before the forge is read again, which may lag — and stay cleared once it is read.
func TestASaidLineClearsWhenItsDeliveryMovesOn(t *testing.T) {
t.Run("a recheck proposes it", func(t *testing.T) {
w, id := saidReady(t)
if _, err := w.h.Recheck(id, "its repo-check status was lost", "a person"); err != nil {
t.Fatal(err)
}
if s := w.state(id); s != Proposed {
t.Fatalf("a recheck left it %s", s)
}
if lines := unansweredOf(w); len(lines) != 0 {
t.Fatalf("a rechecked delivery, proposed again, is still said as unanswered: %+v", lines)
}
w.h.SeeOpenPulls()
if lines := unansweredOf(w); len(lines) != 0 {
t.Fatalf("a rechecked delivery is said again once the forge is read: %+v", lines)
}
})
t.Run("a merge publishes it", func(t *testing.T) {
w, id := saidReady(t)
w.h.WalkMoved(w.ctl.put(aWalk("plan-1", "novox/mesh-controller", merge, true)))
w.h.PullMerged(merged("novox/mesh-controller", 215, readyHeadSHA, merge))
if s := w.state(id); s != Published {
t.Fatalf("a merge left it %s", s)
}
if lines := unansweredOf(w); len(lines) != 0 {
t.Fatalf("a published delivery is still said as unanswered: %+v", lines)
}
w.h.SeeOpenPulls() // the forge's open list lags the merge: the head is still on it
if lines := unansweredOf(w); len(lines) != 0 {
t.Fatalf("a published delivery still on a lagging open list is said: %+v", lines)
}
})
}
// **A said line clears when its pull request leaves the open list** — closed, its event to this owner lost, so the
// delivery is still ready: the forge no longer lists it, and nothing is said of it.
func TestASaidLineClearsWhenItsPullRequestIsNoLongerOpen(t *testing.T) {
w, id := saidReady(t)
w.forge.open = nil
w.h.SeeOpenPulls()
if s := w.state(id); s != Ready {
t.Fatalf("the delivery moved to %s with no event: the case is not the one meant", s)
}
if lines := unansweredOf(w); len(lines) != 0 {
t.Fatalf("a pull request no longer open is still said: %+v", lines)
}
}
modules/mesh-delivery/cmd/mesh-delivery/unchecked.go
+39 -7
View File
@@ -26,6 +26,14 @@ import (
// 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. 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.
//
// A head whose delivery is held in ready or rejected is said by the same rule (novox/hq issue 441). Those states have
// no bound of their own — the verdict is known here — but the forge's status is what lets the pull request merge or
// shows why not, and a status write that lagged or was lost (issue 423), or a check asked again after the verdict,
// leaves a required check pending or never set there: the pull request waits on it, and nothing here asks again. So
// such a head is kept here like one without a delivery, and said on the same bound, as Unanswered — it was announced,
// so never Unannounced, even with no merge check on its head at all. A delivery in any other state is not: proposed
// and checked wait under bounds of their own, and the rest are past the merge, their pull request no longer open.
// 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.
@@ -78,6 +86,15 @@ type unchecked struct {
// asked again from one still waiting on the same ask.
pending map[string]string
first time.Time
// held is the state of the delivery of this head, ready or rejected, when one is held here (novox/hq issue
// 441); empty for a head with no delivery.
held State
}
// saidByTheForge is whether a delivery's head is said here when its required check has not answered on the forge:
// one in ready or rejected, whose state has no bound of its own (novox/hq issue 441). No delivery at all is too.
func saidByTheForge(d *Delivery) bool {
return d == nil || d.State == Ready || d.State == Rejected
}
// unanswered are the merge checks the base requires that the head has no verdict of, each as it reads — "never
@@ -156,13 +173,18 @@ func (h *Holder) SeeOpenPulls() {
continue
}
id := IDOf(p.Owner+"/"+p.Repo, p.HeadSHA)
if h.deliveries[id] != nil {
continue // announced: a delivery with its own bound
d := h.deliveries[id]
if !saidByTheForge(d) {
continue // a delivery with its own bound, or past its merge
}
var held State
if d != nil {
held = d.State
}
// 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}
u := unchecked{pull: p, waiting: waiting, pending: pending, first: now, held: held}
if was, ok := h.unchecked[id]; ok && !askedAgain(was.pending, pending) {
u.first = was.first
} else {
@@ -174,10 +196,11 @@ func (h *Holder) SeeOpenPulls() {
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's check started, rather than was never asked: it has any merge check, or a delivery
// of it is held here, which the forge announced and the controller checked (novox/hq issue 441).
func (u unchecked) started() bool {
_, _, started := u.pull.unanswered()
return started
return started || u.held != ""
}
// bound is how long the head may wait as it does: UncheckedAfter for a check never asked, the table's bound for a
@@ -201,8 +224,9 @@ func (u unchecked) what() string {
func (h *Holder) uncheckedLines(now time.Time) []StalledLine {
var out []StalledLine
for id, u := range h.unchecked {
if h.deliveries[id] != nil || now.Sub(u.first) < u.bound() {
continue
d := h.deliveries[id]
if !saidByTheForge(d) || (d != nil && d.State != u.held) || now.Sub(u.first) < u.bound() {
continue // moved on since the forge was last read: the next reading says it anew
}
h2 := "none: the forge never announced it, so there is no delivery to close — the operator's"
says := fmt.Sprintf("%s/%s#%d is open on %s, which requires the merge check, and its head has had no merge "+
@@ -217,6 +241,14 @@ func (h *Holder) uncheckedLines(now time.Time) []StalledLine {
strings.Join(u.waiting, ", "))
h2 = "none: no delivery of it is held here, so there is none to close — the operator's"
}
if u.held != "" {
// A delivery is held, its verdict known, and the forge shows another (novox/hq issue 441).
says = fmt.Sprintf("%s/%s#%d is open on %s, which requires the merge check, and its delivery is %s here, "+
"but on the forge its check has not answered: %s. The pull request waits on that status and nothing "+
"asks it again. A recheck of the delivery asks its check again, as does a new commit on its branch",
u.pull.Owner, u.pull.Repo, u.pull.Number, u.pull.Base, u.held, strings.Join(u.waiting, ", "))
h2 = "none: its delivery is " + string(u.held) + ", a state the operator moves — the operator's"
}
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, Waiting: waiting})