diff --git a/modules/gitea/test/webhook.test.ts b/modules/gitea/test/webhook.test.ts index cf2450c7..e01ec286 100644 --- a/modules/gitea/test/webhook.test.ts +++ b/modules/gitea/test/webhook.test.ts @@ -350,3 +350,14 @@ test("a pull request's edit event carries its description, and no other pull req assert.equal(eventOf("pull_request", { action, pull_request: pull, repository }).body.body, undefined, action); } }); + +// Issue 1302: a status event carries the status's words, so the delivery gate reads mesh/check's line (the head and the trunk +// it judged) from the event itself. +test("a status event carries its context, state and words", async () => { + const { eventOf } = await import("../webhook.ts"); + const e = eventOf("status", { sha: "a".repeat(40), context: "mesh/check", state: "success", + description: "pass aaaaaaaaaaaa on bbbbbbbbbbbb | unit 3 run, 0 kept", repository: { name: "x", owner: { login: "novox" } } }); + assert.equal(e.name, "forge.status"); + assert.equal((e.body as any).context, "mesh/check"); + assert.equal((e.body as any).description, "pass aaaaaaaaaaaa on bbbbbbbbbbbb | unit 3 run, 0 kept"); +}); diff --git a/modules/gitea/webhook.ts b/modules/gitea/webhook.ts index b0034e94..dba5c3e9 100644 --- a/modules/gitea/webhook.ts +++ b/modules/gitea/webhook.ts @@ -127,7 +127,9 @@ function forgeEventOf(said: string, payload: any): ForgeEvent { return { name: kind, body: { ...base, ref: str(payload.ref), ref_type: str(payload.ref_type), sha: str(payload.sha) } }; } if (kind === "status") { - return { name: "status", body: { ...base, sha: str(payload.sha), context: str(payload.context), state: str(payload.state) } }; + // Its words too (issue 1302): mesh/check's line names the head and the trunk it judged, which the delivery gate reads. + return { name: "status", body: { ...base, sha: str(payload.sha), context: str(payload.context), state: str(payload.state), + description: str(payload.description) } }; } const name = family === "other" ? `other.${kind.replace(/[^a-z0-9_-]/gi, "_") || "unknown"}` : ["fork"].includes(family) ? family : `${family}.${action ?? "changed"}`; diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/checks.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/checks.go index fab99083..b0c33f30 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/checks.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/checks.go @@ -18,6 +18,7 @@ import ( // Context names of the statuses this owner can say more of than the forge keeps. const ( + // ctxGate is the controller's merge gate, kept where a base still requires it; mesh/check is ctxCheck (onestatus.go). ctxGate = "mesh/merge-gate" ctxRepo = "mesh/repo-check" ctxDelivery = "mesh/delivery" @@ -230,6 +231,10 @@ func (h *Holder) addWhatIsKnown(s *CheckStatus, v *Verdict, d *checksDelivery) { } else if v.Repo != "" { s.Verdict = strings.ToUpper(v.Repo) + " — " + v.RepoSaid } + case ctxCheck: + if v != nil && v.Status == ctxCheck { + s.Verdict = strings.ToUpper(firstOf(v.Gate, "error")) + " — " + v.Summary + } case ctxDelivery: if d != nil { s.Verdict = string(d.State) diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/configheld.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/configheld.go index 3d40e5bc..51a03475 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/configheld.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/configheld.go @@ -103,6 +103,13 @@ func (h *Holder) askProposedAgain(id, why string) { h.mu.Unlock() return } + if d.judgedByCheck() { + // The checker's (issue 1302): its check is not asked here, and whether a held config value is weighed is the + // checker's composition's. + h.Logf("[mesh-delivery] %s: a held config value changed; %v", id, checkerRefusal(id)) + h.mu.Unlock() + return + } owner, repo, commit, page, number := d.Owner(), d.Repo(), d.Commit, d.HTMLURL, d.Number h.mu.Unlock() if err := h.checkingAgain(owner, repo, number, commit, page, why); err != nil { diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/events.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/events.go index 8eb4bab5..e8f6c0f2 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/events.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/events.go @@ -19,6 +19,9 @@ type ForgeStatusEvent struct { SHA string `json:"sha"` Context string `json:"context"` State string `json:"state"` + // Description is the status's words (the gitea module's status event carries them since issue 1302): mesh/check's + // line names the head and the trunk it judged. + Description string `json:"description,omitempty"` } // wantForge says the forge's open pull requests are to be read at the next look: a check may have answered. @@ -44,7 +47,8 @@ func (h *Holder) AnswersChanged() { } } -// CommitStatus takes a commit status set on the forge: one of the merge checks (mesh/merge-gate, mesh/repo-check) says a +// CommitStatus takes a commit status set on the forge: one of the merge checks (mesh/check; hq's mesh/repo-check and, on a +// base that still requires it, mesh/merge-gate) says a // check started, answered, or was put back, so the forge's open pull requests are read to see which heads still wait on // one (SeeOpenPulls). A status of any other context shows nothing the merge needs. // @@ -60,6 +64,10 @@ func (h *Holder) CommitStatus(e ForgeStatusEvent) { h.wantForge() h.mu.Lock() defer h.mu.Unlock() + if e.Context == ctxCheck && (e.Description != "" || e.State == "pending") { + // mesh/check is the verdict on its base (issue 1302): taken at once, as the controller's `checked` was. + h.takeCheckStatus(e.Owner+"/"+e.Repo, e.SHA, e.State, e.Description) + } if e.State != "success" { h.withdrawForStatus(e) return diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go index fa731a8e..b3660c23 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go @@ -78,6 +78,9 @@ type Holder struct { refusals []string // unchecked are the open pull requests' heads seen without a merge check (unchecked.go). unchecked map[string]unchecked + // checker are the checker's verdicts by the hash of the heads they judged, as its verdicts bucket holds them + // (onestatus.go, issue 1302). + checker map[string]checkerVerdict // walks are the walks' and batches' last records (answers.go); orphans the waiting walks no live delivery // holds, by when that was first seen; turning the walks whose turn came this look, for some deliveries. walks map[string]seenWalk @@ -170,6 +173,7 @@ func (h *Holder) init() { h.lostRead = map[string]time.Time{} h.walkAsked = map[string]time.Time{} h.unchecked = map[string]unchecked{} + h.checker = map[string]checkerVerdict{} h.walks = map[string]seenWalk{} h.orphans = map[string]time.Time{} h.asking = map[string]bool{} @@ -1959,7 +1963,7 @@ func standingVerdict(p OpenPull, now time.Time) *Verdict { if p.Checks == nil { return nil } - contexts := slices.Clone(mergeChecks) + contexts := []string{"mesh/merge-gate", "mesh/repo-check"} // the controller's; mesh/check is taken in askChecks for _, c := range p.Required { if !slices.Contains(contexts, c) { contexts = append(contexts, c) @@ -1995,7 +1999,8 @@ func (h *Holder) askChecks(now time.Time) func() { } w := want{id: d.ID, head: d.Commit, number: d.Number, member: MemberOf(d)} w.owner, w.repo, _ = strings.Cut(d.Repository, "/") - if !askedCheck(d) { + // A base that requires mesh/check is the checker's: nothing is asked of the controller (issue 1302). + if !askedCheck(d) && !d.judgedByCheck() { if tried, was := h.askTried[d.ID]; was && now.Sub(tried) < time.Minute { h.lookAt(tried.Add(time.Minute), "a proposed delivery's check that could not be asked") } else { @@ -2014,14 +2019,30 @@ func (h *Holder) askChecks(now time.Time) func() { open, err := h.Forge.OpenPulls() for _, w := range wants { var standing *Verdict + oneStatus := false if err == nil { for _, p := range open { if p.Number == w.number && strings.EqualFold(p.Owner, w.owner) && strings.EqualFold(p.Repo, w.repo) && sameCommit(p.HeadSHA, w.head) { + if slices.Contains(p.Required, ctxCheck) { + // Its base requires mesh/check (issue 1302): its verdict is that status, when it stands, and + // nothing is asked of the controller. + oneStatus = true + h.mu.Lock() + h.noteRequired(h.deliveries[w.id], p.Required) + if c := p.Checks[ctxCheck]; c.State != "" { + h.takeCheckStatus(w.owner+"/"+w.repo, w.head, c.State, c.Description) + } + h.mu.Unlock() + continue + } standing = standingVerdict(p, h.Now()) } } } + if oneStatus { + continue + } if standing != nil { h.mu.Lock() if d := h.deliveries[w.id]; d != nil && d.State == Proposed && d.Check == nil { @@ -2169,14 +2190,20 @@ func (h *Holder) planGroup(g *Group, members []*Delivery, now time.Time) []func( allReady = false } } - again := g.Check != nil && (!sameMembers(g.Check.Commits, commits) || g.Check.OutOfDate != "" && rejudgedSince(members, g.Check.OutOfDateAt)) + // A group judged by mesh/check (issue 1302, ADR 0447 §2): its verdict is the checker's one verdict at its members' + // heads, read, not asked; nothing below asks a composed check or owes a member's mesh/merge-gate. + oneStatus := oneStatusGroup(members) + if oneStatus { + h.composeFromChecker(g, members, now) + } + again := !oneStatus && g.Check != nil && (!sameMembers(g.Check.Commits, commits) || g.Check.OutOfDate != "" && rejudgedSince(members, g.Check.OutOfDateAt)) // A closed group's member whose merge the tool refused, after the composed check answered, is judged again at once // (novox/hq ADR 0418 §4, issue 1082): the refusal is a sibling's merge having moved what the verdict stands on, and // nothing else asks the composed check again. Not while a check is on its way (its verdict is empty then). // **A refusal that names the member's OWN pending status** (mesh/repo-check, with the gate not pending: the composed // check set it) is not that: the composed check sets no repository check, so asking it again changes nothing. The // member's own check is asked, once for the head (issue 1049); its verdict sets the status. - if !again && g.Closed && g.Check != nil && g.Check.Verdict != "" { + if !oneStatus && !again && g.Closed && g.Check != nil && g.Check.Verdict != "" { for _, d := range members { if !refusedAfter(d, g.Check.At) { continue @@ -2193,7 +2220,7 @@ func (h *Holder) planGroup(g *Group, members []*Delivery, now time.Time) []func( break } } - if allReady && (g.Check == nil && !g.Closed || again) { + if !oneStatus && allReady && (g.Check == nil && !g.Closed || again) { ms := membersOfAsk(members) id := g.ID asks = append(asks, func() { @@ -2262,7 +2289,7 @@ func (h *Holder) planGroup(g *Group, members []*Delivery, now time.Time) []func( } // Each member's mesh/merge-gate is the composed check's (ADR 0403 §1, §6): pending, saying why, while no composed // verdict stands at these heads. The verdict itself sets it, success or failure, in the forge module. - if why := composedPending(g, members); why != "" { + if why := composedPending(g, members); why != "" && !oneStatus { if g.SaidGate == nil { g.SaidGate = map[string]string{} } @@ -2820,8 +2847,8 @@ func ownStatusPending(answer string) bool { // its own. Under the lock; the ask is made outside it. Nil when it was asked already for this head. func (h *Holder) askOwnAfterRefusal(d *Delivery) func() { r := d.Ready - if !r.OwnAskedAt.IsZero() { - return nil + if !r.OwnAskedAt.IsZero() || d.judgedByCheck() { + return nil // asked already; or the checker's, who checks it again on its own (issue 1302) } r.OwnAskedAt = h.Now() h.keep(d) diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/lostcheck.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/lostcheck.go index 07725fa8..bef8e0d7 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/lostcheck.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/lostcheck.go @@ -30,8 +30,8 @@ func (h *Holder) lostCheckBound(repository string) time.Duration { // awaitedSince is when a proposed delivery's check was asked, when it waits for one this owner asked: no verdict yet, // or a verdict out of date with its check against the trunk's head asked. False for a delivery waiting for none. func awaitedSince(d *Delivery) (time.Time, bool) { - if d.State != Proposed || d.MergedAs != "" || d.Number == 0 { - return time.Time{}, false + if d.State != Proposed || d.MergedAs != "" || d.Number == 0 || d.judgedByCheck() { + return time.Time{}, false // a head on mesh/check asks the controller nothing (issue 1302) } switch { case d.Check == nil && askedCheck(d): diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/main.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/main.go index e5ef06db..79e58504 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/main.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/main.go @@ -115,6 +115,25 @@ func run(h *Holder, l *listening, occupancy *busOccupancy) { time.Sleep(wait) } }() + // The checker's verdicts (issue 1302, ADR 0447 §2): the Approve on mesh/check is asked only on its verdict at exactly the + // heads an ask covers, and a group's verdict is that one; each change asks for a look. + go func() { + for wait := 2 * time.Second; ; wait = min(wait*2, time.Minute) { + err := stdio.State(checkerVerdicts).Watch("", func(c stdio.StateChange) error { + if c.Op == "delete" { + h.CheckerVerdictChanged(c.Key, nil) + } else { + h.CheckerVerdictChanged(c.Key, c.Value) + } + return nil + }) + if err == nil { + return + } + logf("[mesh-delivery] not hearing the checker's verdicts yet (%v); asking again in %s", err, wait) + time.Sleep(wait) + } + }() logf("[mesh-delivery] listening: %s", strings.Join(followed, ", ")) } @@ -313,7 +332,8 @@ func tools(h *Holder, l *listening) []stdio.Tool { {Name: seat + "groups", Description: "Every delivery group: its members, its composed check and its state, derived from its members. " + "A group has no order: it is merged in one pass and delivered by one plan, whose tiers come from the module " + - "graph (novox/hq ADR 0418). composed_gate says its members' gate is the composed check's (ADR 0403).", + "graph (novox/hq ADR 0418). On mesh/check its check is the checker's one verdict at every member's head (ADR 0447); " + + "composed_gate says its members' gate is the controller's composed check's (ADR 0403), for a base still on it.", Input: map[string]any{"type": "object", "properties": map[string]any{ "all": map[string]any{"type": "string", "enum": []string{"true", "false"}}}}, Run: func(a map[string]any) (any, error) { return h.Groups(boolArg(a, "all")), nil }}, @@ -330,7 +350,7 @@ func tools(h *Holder, l *listening) []stdio.Tool { }}, {Name: seat + "checks", Description: "What the mesh's checks said of a pull request's head or of one commit: each of the commit's " + - "statuses (mesh/merge-gate, mesh/repo-check, mesh/delivery, …) with its state, description and when it " + + "statuses (mesh/check, mesh/delivery, …; hq's mesh/repo-check) with its state, description and when it " + "was set; the merge check's full verdict as the controller said it — each layer's summary, the machine " + "that ran it, when, its build id and its report; and whether the branch's protection would let it " + "merge, every required status being success.", diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/manifest_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/manifest_test.go index 4a9c41bc..029a5dd7 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/manifest_test.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/manifest_test.go @@ -83,10 +83,10 @@ func TestItClaimsTheDeliverySeatAndFollowsWhatItHandles(t *testing.T) { if !reflect.DeepEqual(buckets, []string{"deliveries", "groups", "pulls", "asks", "merges", "owed", "source"}) { t.Fatalf("state %v", buckets) } - // What it reads: the carry from today's module's three buckets, the teller's answers, the seat's holder and the - // machines' haves (ADR 0450 §6 and its insight of 2026-10-20). + // What it reads: the carry from today's module's three buckets, the teller's answers, the checker's verdicts (issue + // 1302), the seat's holder and the machines' haves (ADR 0450 §6 and its insight of 2026-10-20). if !reflect.DeepEqual(m.Reads, []string{"mesh-delivery.deliveries", "mesh-delivery.groups", "mesh-delivery.pulls", - "mesh-delivery-teller.answers", "mesh-controller.assignments", "node-engine.have"}) { + "mesh-delivery-teller.answers", "mesh-delivery-checker.verdicts", "mesh-controller.assignments", "node-engine.have"}) { t.Fatalf("reads %v", m.Reads) } // It calls exactly the controller's seven verbs (ADR 0450 §7) and reads the forge with three tools; the forge's diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/model.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/model.go index f025d9a9..5eb6cef3 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/model.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/model.go @@ -54,6 +54,12 @@ type Delivery struct { Plan *DeliveryPlan `json:"plan,omitempty"` // Check is its own check's verdict. Check *Verdict `json:"check,omitempty"` + // Required are the statuses its base requires, as the forge last said them (gitea_open_pulls): mesh/check makes it + // the checker's (issue 1302). Empty until the forge said them. + Required []string `json:"required,omitempty"` + // CheckState is its head's mesh/check as the forge last said it (pending, success, failure, error): a pending one + // after a pass is the checker judging it again, and no Approve is asked meanwhile (issue 1302). + CheckState string `json:"check_state,omitempty"` // CheckAsk is this owner's own ask of its check, when the forge's announcement did not bring one: a head // adopted while it already waited, or one whose announcement the controller missed. CheckAsk *CheckAsk `json:"check_ask,omitempty"` @@ -179,6 +185,9 @@ type Verdict struct { Trunk string `json:"trunk,omitempty"` Beside map[string]string `json:"beside,omitempty"` Took string `json:"took,omitempty"` + // Status is the forge status it was read from when it is not the controller's verdict: mesh/check, the checker's + // (novox/hq ADR 0447, issue 1302). Empty for the controller's `checked`. + Status string `json:"status,omitempty"` } // Judged is what a verdict judged, in words: "head 1a2b3c4d5e6f merged into trunk 7a8b9c0d1e2f" (ADR 0323 §3); @@ -1034,7 +1043,8 @@ type Group struct { Updated time.Time `json:"updated"` // Check is its composed check: asked of the controller for these member commits, and its verdict. Check *GroupCheck `json:"check,omitempty"` - // SaidGate is, by member id, why the member's mesh/merge-gate was last said pending: written again only when it + // SaidGate is, by member id, why the member's mesh/merge-gate was last said pending (a group the controller judges; + // a group on mesh/check is owed none, issue 1302): written again only when it // changes (novox/hq ADR 0403 §1, §6, issue 978). Cleared once a composed verdict stands at the heads, which the // forge module then sets as every member's gate. SaidGate map[string]string `json:"said_gate,omitempty"` diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/onestatus.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/onestatus.go new file mode 100644 index 00000000..35e7ba14 --- /dev/null +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/onestatus.go @@ -0,0 +1,289 @@ +package main + +import ( + "encoding/json" + "fmt" + "regexp" + "slices" + "sort" + "strings" + "time" + + sdk "git.novox.be/novox/mesh-sdk/go/delivery" +) + +// **One required status** (novox/hq ADR 0447, issue 1302; ADR 0503 §3, §5). +// +// Main requires `mesh/check` everywhere but hq: the checker's verdict on a pull request's head merged onto its trunk's +// head, written by its forge bridge. On such a base this owner reads that one status, and nothing of the controller: +// +// - the verdict of a head is its mesh/check, heard as the forge's status event (its words name the head and the trunk +// it judged: "pass on | …") or read from the forge's open pull requests; it is kept as the +// delivery's Check, marked as mesh/check's, and moves the delivery as the controller's verdict did; +// - the Approve is asked only when the checker's verdict for exactly the heads the ask covers passed (its verdicts +// bucket, keyed by the heads' hash, ADR 0447 §2): a member's pass from a check that judged it alone, before a sibling +// joined, is not the group's; +// - a group's verdict is that one verdict (no composed check is asked, and no member is owed a mesh/merge-gate); a +// member's mesh/check that is no longer a success withdraws the group's ask (ADR 0441 §1, withdrawForStatus); +// - no check is asked of the controller: the checker checks every head on every move, and the trunk's move is its to +// say on mesh/check (ADR 0447 §4). The controller is asked only for a base that requires mesh/repo-check without +// mesh/check (hq), and only what it was asked before (the lead's decision for issue 1302 in R4e). +// +// A base whose required statuses are not known yet is read as before: the forge's open pull requests say them at the next +// look, and a mesh/check heard on the head says it too. + +// ctxCheck is the one status main requires (ADR 0447 §1). +const ctxCheck = "mesh/check" + +// checkerVerdicts is the checker's verdicts bucket as this module reads it (its manifest's reads). +const checkerVerdicts = "mesh-delivery-checker.verdicts" + +// The states of a checker's verdict (mesh-sdk go/delivery VerdictRunning, VerdictPass, VerdictFail; this module's mesh-sdk +// pin is older than them). +const ( + checkerRunning = "running" + checkerPass = "pass" +) + +// checkerVerdict is a verdict of the checker as its verdicts bucket keeps it (mesh-sdk go/delivery Verdict, read here by +// the fields the gate needs): the group, the hash of the heads it judged, each repository's trunk head and its state. +type checkerVerdict struct { + Group string `json:"group"` + Heads string `json:"heads"` + Trunk map[string]string `json:"trunk,omitempty"` + State string `json:"state"` + Failed struct { + Test string `json:"test,omitempty"` + Machine string `json:"machine,omitempty"` + Words string `json:"words,omitempty"` + } `json:"failed,omitzero"` +} + +// CheckerVerdictChanged takes a key of the checker's verdicts bucket (`.`) put or deleted (an empty +// value): the view of the checker's verdicts the Approve and a group's verdict are read from. A value that is no verdict +// is said and dropped. +func (h *Holder) CheckerVerdictChanged(key string, value []byte) { + h.mu.Lock() + defer h.mu.Unlock() + if h.checker == nil { + h.checker = map[string]checkerVerdict{} + } + hash := key + if i := strings.LastIndex(key, "."); i >= 0 { + hash = key[i+1:] + } + if len(value) == 0 || string(value) == "null" { + delete(h.checker, hash) + h.Poke() + return + } + var v checkerVerdict + if err := json.Unmarshal(value, &v); err != nil || v.State == "" { + h.Logf("[mesh-delivery] the checker's verdict %s cannot be read (%v); it is not taken", key, err) + return + } + if v.Heads == "" { + v.Heads = hash + } + h.checker[v.Heads] = v + h.Poke() +} + +// checkWords are the head and the trunk mesh/check's line names: " on " (ADR 0447 §3). +var checkWords = regexp.MustCompile(`\b([0-9a-f]{12}) on ([0-9a-f]{12})\b`) + +// checkLine is the head and the trunk commit a mesh/check line names, twelve characters each; empty when it names none. +func checkLine(desc string) (head, trunk string) { + m := checkWords.FindStringSubmatch(strings.ToLower(desc)) + if m == nil { + return "", "" + } + return m[1], m[2] +} + +// verdictOfCheck is a delivery's verdict from its head's mesh/check: its state as a verdict, its line as the summary, and +// the head and trunk it names. +func verdictOfCheck(state, desc string, now time.Time) *Verdict { + head, trunk := checkLine(desc) + return &Verdict{At: now, Gate: verdictOfState(state), Summary: firstOf(desc, "mesh/check "+state), Head: head, Trunk: trunk, + Status: ctxCheck} +} + +// judgedByCheck is whether the delivery's base requires mesh/check, as the forge said it, or its verdict is mesh/check's. +func (d *Delivery) judgedByCheck() bool { + return slices.Contains(d.Required, ctxCheck) || (d.Check != nil && d.Check.Status == ctxCheck) +} + +// noteRequired keeps what the forge said the delivery's base requires. Under the lock. +func (h *Holder) noteRequired(d *Delivery, required []string) { + if d == nil || required == nil || slices.Equal(d.Required, required) { + return + } + d.Required = slices.Clone(required) + h.keep(d) +} + +// takeCheckStatus takes a head's mesh/check as its delivery's verdict, as the controller's `checked` was taken: a +// proposed delivery moves on it, a rejected one judged again against another trunk or now passing is proposed again +// first. A pending mesh/check is no verdict and changes nothing here (its ask is withdrawn by withdrawForStatus). The same +// verdict heard twice changes nothing. Under the lock. +func (h *Holder) takeCheckStatus(repository, sha, state, desc string) { + d := h.deliveries[IDOf(strings.ToLower(repository), sha)] + if d == nil || state == "" { + return // its announcement makes it; the forge's open pull requests bring the verdict then + } + if d.CheckState != state { + d.CheckState = state + h.keep(d) + } + if state == "pending" { + return + } + if !slices.Contains(d.Required, ctxCheck) { + d.Required = append(d.Required, ctxCheck) + } + now := h.Now() + v := verdictOfCheck(state, desc, now) + if c := d.Check; c != nil && c.Status == ctxCheck && c.Gate == v.Gate && c.Trunk == v.Trunk && c.Summary == v.Summary { + return + } + if d.State == Rejected && d.Check != nil && (v.Passes() || v.Trunk != "" && !sameCommit(d.Check.Trunk, v.Trunk)) { + t, err := Apply(d, EvRecheck, true, Facts{Now: now, By: byOwner, Why: fmt.Sprintf("its mesh/check judged it again: %s", + oneLine(clip(desc, 120)))}) + if err != nil { + h.refuse(err) + return + } + h.moved(d, []Transition{t}) + } + if d.State != Proposed { + h.keep(d) + return // past its verdict: the merge reads the forge's status itself + } + d.Check, d.AskedAgain, d.CheckAsk = v, nil, nil + h.mergeAgainNow(d) + if why := d.outOfDate(); why != "" { + h.Logf("[mesh-delivery] %s: its mesh/check is %s; the checker checks it again", d.ID, why) + } + h.Logf("[mesh-delivery] %s: its mesh/check is %s — %s", d.ID, state, oneLine(clip(desc, 120))) + h.settleAndKeep(d) +} + +// headsOf are the heads the checker judges together for an ask: the members the ask covers. +func headsOf(members []*Delivery) []sdk.Head { + out := make([]sdk.Head, 0, len(members)) + for _, m := range members { + out = append(out, sdk.Head{Repository: strings.ToLower(m.Repository), Number: m.Number, Head: strings.ToLower(m.Commit)}) + } + return out +} + +// checkerAt is the checker's verdict for exactly these members' heads, and their hash. +func (h *Holder) checkerAt(members []*Delivery) (checkerVerdict, string, bool) { + hash, err := sdk.HeadsHash(headsOf(members)) + if err != nil { + return checkerVerdict{}, "", false + } + v, ok := h.checker[hash] + return v, hash, ok +} + +// checkerWaits is why the Approve of a delivery on mesh/check is not asked yet: the checker's verdict for the heads the ask +// covers is not held, or did not pass. Empty when it passed, for a delivery not judged by mesh/check, and once an ask +// covers the head (the merge then reads the forge's statuses itself). Under the lock. +func (h *Holder) checkerWaits(d *Delivery) string { + if !d.judgedByCheck() || h.covering(d) != nil { + return "" + } + members := h.askMembers(d) + for _, m := range members { + if m.CheckState != "" && m.CheckState != "success" { + return fmt.Sprintf("the mesh/check of %s is %s at its head %s", m.ID, m.CheckState, shortOf(m.Commit, 12)) + } + } + v, hash, ok := h.checkerAt(members) + what := "its head" + if len(members) > 1 { + what = fmt.Sprintf("its group's %d heads", len(members)) + } + switch { + case !ok: + return fmt.Sprintf("the checker's verdict at %s (heads %s) is not held yet", what, hash) + case v.State == checkerRunning: + return fmt.Sprintf("the checker's verdict at %s (heads %s) is running", what, hash) + case v.State != checkerPass: + return fmt.Sprintf("the checker's verdict at %s (heads %s) is %s", what, hash, v.State) + } + return "" +} + +// oneStatusGroup is whether a group is judged by mesh/check: one of its members is. +func oneStatusGroup(members []*Delivery) bool { + return slices.ContainsFunc(members, (*Delivery).judgedByCheck) +} + +// composeFromChecker makes a group's verdict on mesh/check from the checker's one verdict at its members' heads (ADR 0447 +// §2): no composed check is asked. It is kept in the group's check as a composed verdict was, so the group's state, its +// Approve and its merges read it unchanged: running or not held is no verdict yet, a pass or a failure is the group's, and +// a member whose trunk moved past what it judged puts it out of date. Under the lock. +func (h *Holder) composeFromChecker(g *Group, members []*Delivery, now time.Time) { + live := make([]*Delivery, 0, len(members)) + for _, d := range members { + if d.MergedAs == "" && !d.State.Final() { + live = append(live, d) + } + } + if len(live) == 0 || (g.Closed && g.Check != nil && g.Check.Verdict != "") { + return // every member on the trunk, or a closed group: the verdict its Approve was asked on stands + } + v, hash, ok := h.checkerAt(live) + c := &GroupCheck{Asked: "mesh/check " + hash, At: now, Commits: commitsOf(members)} + if g.Check != nil && g.Check.Asked == c.Asked && sameMembers(g.Check.Commits, c.Commits) { + c.At = g.Check.At + } + if ok && v.State != checkerRunning { + c.Verdict = v.State + c.Summary = "the checker's verdict at the members' heads (" + hash + ")" + if w := firstOf(v.Failed.Test, v.Failed.Machine, v.Failed.Words); w != "" { + c.Summary += ": " + w + } + c.Trunks = map[string]string{} + ids := make([]string, 0, len(live)) + for _, d := range live { + ids = append(ids, d.ID) + } + sort.Strings(ids) + for _, id := range ids { + d := h.deliveries[id] + judged := v.Trunk[strings.ToLower(d.Repository)] + if judged == "" { + continue + } + c.Trunks[d.ID] = judged + if c.OutOfDate == "" && d.TrunkNow != "" && !sameCommit(judged, d.TrunkNow) { + c.OutOfDate = fmt.Sprintf("the checker's verdict is out of date: %s was judged on trunk %s, the trunk is now %s", + d.ID, shortOf(judged, 12), shortOf(d.TrunkNow, 12)) + c.OutOfDateAt = now + } + } + } + if g.Check != nil && g.Check.Asked == c.Asked && g.Check.Verdict == c.Verdict && g.Check.OutOfDate == c.OutOfDate && + sameMembers(g.Check.Commits, c.Commits) { + return + } + g.Check, g.Updated = c, now + h.keepGroup(g) + if c.Verdict != "" { + for _, d := range members { + h.mergeAgainNow(d) + } + h.Logf("[mesh-delivery] group %s: %s — %s", g.ID, c.Verdict, c.Summary) + } +} + +// checkerRefusal is why a delivery on mesh/check is asked nothing of the controller: the checker checks it. +func checkerRefusal(id string) error { + return fmt.Errorf("%s is judged by mesh/check, the checker's: the checker checks its head again on every move of the head, "+ + "its trunk or its group, and nothing is asked of it here (novox/hq ADR 0447). Read where its check stands with the "+ + "checker's checker_pull tool", id) +} diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/onestatus_bus_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/onestatus_bus_test.go new file mode 100644 index 00000000..0c44c294 --- /dev/null +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/onestatus_bus_test.go @@ -0,0 +1,82 @@ +//go:build integration + +package main + +import ( + "context" + "encoding/json" + "strings" + "testing" + "time" + + sdk "git.novox.be/novox/mesh-sdk/go/delivery" +) + +// **Issue 1302's acceptance over a real bus** (ADR 0447, 1302-S10): a pull request into a main that requires mesh/check goes +// ready on its mesh/check alone — the forge's status event and the checker's verdict in its verdicts bucket — has its Approve +// asked, and on the Approve its merge is handed to the bridge; nothing is asked of the controller (no delivery-check, no +// forget-checks) and no controller verdict is heard at all. +func TestADeliveryOnMeshCheckGoesReadyAskedAndMergedOverARealBus(t *testing.T) { + b := newTestBus(t) + ctx := context.Background() + rt := b.start(t) + evctx, cancel := context.WithCancel(ctx) + defer cancel() + rt.deliverEvents(evctx) + rt.until("the module to subscribe to the events it follows", 20*time.Second, func() bool { + rt.mu.Lock() + defer rt.mu.Unlock() + return len(rt.subs) >= len(followed) + }) + rt.untilHolding() + + const head = "2222222222222222222222222222222222222222" + const trunk = "3333333333333333333333333333333333333333" + rt.mu.Lock() + rt.openPulls = []OpenPull{{Owner: "novox", Repo: "app", Number: 8, Base: "main", HeadSHA: head, Required: []string{ctxCheck}, + Statuses: []string{}, Checks: map[string]HeadCheck{}}} + rt.mu.Unlock() + announced := pr("novox/app", 8, head, "fix/y", "src/b.go") + announced.Body = "Reviewed-by: lead@shanks at " + head + b.publish(t, "mesh.mod.gitea.event.pull.updated", announced) + + // The checker's verdict at the head, then the forge's word that its bridge set mesh/check. + hash, err := sdk.HeadsHash([]sdk.Head{{Repository: "novox/app", Number: 8, Head: head}}) + if err != nil { + t.Fatal(err) + } + verdict, _ := json.Marshal(map[string]any{"v": 1, "group": "fix/y", "heads": hash, "trunk": map[string]string{"novox/app": trunk}, + "state": "pass"}) + if _, err := b.kvs[checkerVerdicts].Put(ctx, "fix/y."+hash, verdict); err != nil { + t.Fatal(err) + } + line := "pass " + head[:12] + " on " + trunk[:12] + " | unit 3 run, 0 kept" + rt.mu.Lock() + rt.openPulls[0].Statuses = []string{ctxCheck} + rt.openPulls[0].Checks = map[string]HeadCheck{ctxCheck: {State: "success", Description: line}} + rt.mu.Unlock() + set := b.publish(t, "mesh.mod.gitea.event.forge.status", map[string]any{"owner": "novox", "repo": "app", "sha": head, + "context": ctxCheck, "state": "success", "description": line}) + + var key string + rt.until("the ask to be written on mesh/check alone", 10*time.Second, func() bool { key = onlyAsk(b); return key != "" }) + t.Logf("the ask was written %s after mesh/check was set", time.Since(set)) + b.answer(t, key, OptionApprove) + var handed sdk.Merge + rt.until("the merge to be handed to the bridge", 10*time.Second, func() bool { handed = mergeAt(b, "fix/y"); return len(handed.Pulls) > 0 }) + if len(handed.Pulls) != 1 || handed.Pulls[0].Head.Head != head || handed.Pulls[0].State != sdk.MergeMerging { + t.Fatalf("the merge handed is %+v", handed) + } + for _, verb := range []string{"delivery-check"} { + if n := len(rt.asksOf("seat:" + ControllerSeat + "." + verb)); n != 0 { + t.Fatalf("the controller was asked %s %d times on a mesh/check base", verb, n) + } + } + // No mesh/merge-gate is owed on the head. + keys, _ := b.kvs["owed"].Keys(ctx) + for _, k := range keys { + if strings.Contains(k, "merge-gate") { + t.Fatalf("a mesh/merge-gate was owed: %s", k) + } + } +} diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/onestatus_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/onestatus_test.go new file mode 100644 index 00000000..b5f17590 --- /dev/null +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/onestatus_test.go @@ -0,0 +1,234 @@ +package main + +import ( + "encoding/json" + "strings" + "testing" + "time" + + sdk "git.novox.be/novox/mesh-sdk/go/delivery" +) + +// The tests of issue 1302 (ADR 0447 §2, §4; ADR 0441 §1; ADR 0421 §1, §3; lead decisions L6 and the gate's part of +// 1318): on a base that requires mesh/check the gate reads that one status. A delivery is ready, its Approve asked and its +// merge handed on when mesh/check passed at its head and the checker's verdict covers every member's head as they are; no +// check is asked of the controller, no composed check is asked, and no mesh/merge-gate is owed. + +const ( + oneRepo, oneRepo2 = "novox/mesh-sdk", "novox/mesh-delivery" + oneHead, oneHead2 = "e5e5e5e5e5e5e5e5e5e5e5e5e5e5e5e5e5e5e5e5", "f6f6f6f6f6f6f6f6f6f6f6f6f6f6f6f6f6f6f6f6" + oneTrunk = "a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1" + oneTrunk2 = "b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2" +) + +// requiresCheck makes the forge say each pull request's base requires mesh/check alone, as gitea_open_pulls answers. +func requiresCheck(w *world, pulls ...PullEvent) { + w.forge.mu.Lock() + defer w.forge.mu.Unlock() + w.forge.open = nil + for _, p := range pulls { + w.forge.open = append(w.forge.open, OpenPull{Owner: p.Owner, Repo: p.Repo, Number: p.Number, Base: "main", + HeadSHA: p.HeadSHA, Required: []string{ctxCheck}, Statuses: []string{}, Checks: map[string]HeadCheck{}}) + } +} + +// checkStatus is the forge's word that mesh/check was set on a head, with its line as the checker writes it. +func checkStatus(repository, head, state, trunk string) ForgeStatusEvent { + owner, repo, _ := strings.Cut(repository, "/") + words := map[string]string{"success": "pass", "pending": "checking", "failure": "failed: a test"}[state] + return ForgeStatusEvent{Owner: owner, Repo: repo, SHA: head, Context: ctxCheck, State: state, + Description: words + " " + head[:12] + " on " + trunk[:12] + " | unit 4 run, 0 kept"} +} + +// putCheckerVerdict puts the checker's verdict for these heads in the gate's view of the checker's verdicts bucket. +func putCheckerVerdict(w *world, state string, heads ...sdk.Head) { + w.t.Helper() + hash, err := sdk.HeadsHash(heads) + if err != nil { + w.t.Fatal(err) + } + trunk := map[string]string{} + for _, h := range heads { + trunk[h.Repository] = oneTrunk + } + value, _ := json.Marshal(map[string]any{"v": 1, "group": "g", "heads": hash, "trunk": trunk, "state": state}) + w.h.CheckerVerdictChanged("g."+hash, value) +} + +func headOf(repository string, number int, head string) sdk.Head { + return sdk.Head{Repository: repository, Number: number, Head: head} +} + +func mergeGateOwed(w *world) []string { + w.forge.mu.Lock() + defer w.forge.mu.Unlock() + var out []string + for k := range w.forge.statuses { + if strings.HasSuffix(k, " mesh/merge-gate") { + out = append(out, k) + } + } + return out +} + +// 1302 acceptance: the gate's table moves a lone delivery to ready on mesh/check alone, asks its Approve and merges it, +// with no check asked of the controller and no mesh/merge-gate written. +func TestALoneDeliveryGoesReadyAskedAndMergedOnMeshCheckAlone(t *testing.T) { + w := newWorld(t) + p := pr(oneRepo, 1, oneHead, "feat/one") + requiresCheck(w, p) + w.h.PullUpdated(p) + w.settleAll() + if got := w.ctl.checks; len(got) != 0 { + t.Fatalf("a check was asked of the controller for a mesh/check base: %v", got) + } + id := IDOf(oneRepo, oneHead) + if s := w.state(id); s != Proposed { + t.Fatalf("before mesh/check it is %s, want proposed", s) + } + putCheckerVerdict(w, "pass", headOf(oneRepo, 1, oneHead)) + w.h.CommitStatus(checkStatus(oneRepo, oneHead, "success", oneTrunk)) + w.settleAll() + if s := w.state(id); s == Proposed || s == Rejected { + t.Fatalf("after mesh/check passed it is %s (%s), want ready or on", s, w.logs()) + } + if d := w.delivery(id); d.Check == nil || d.Check.Status != ctxCheck || d.Check.Trunk != oneTrunk[:12] || d.Check.Head != oneHead[:12] { + t.Fatalf("the verdict is not mesh/check's: %+v", d.Check) + } + if w.op.count() != 1 { + t.Fatalf("its Approve was asked %d times, want once", w.op.count()) + } + if len(w.forge.merges) != 1 || !strings.Contains(w.forge.merges[0], "mesh-sdk#1") { + t.Fatalf("merges %v, want mesh-sdk#1", w.forge.merges) + } + if got := mergeGateOwed(w); len(got) != 0 { + t.Fatalf("mesh/merge-gate was written: %v", got) + } + if len(w.ctl.checks) != 0 { + t.Fatalf("a check was asked of the controller: %v", w.ctl.checks) + } +} + +// 1302 (lead: the verdict read must be the group's): mesh/check passing on a head is not enough while the checker's +// verdict for exactly the delivery's heads is not held: the Approve waits for it, and says so. +func TestTheApproveWaitsForTheCheckersVerdictAtTheDeliverysHeads(t *testing.T) { + w := newWorld(t) + p := pr(oneRepo, 1, oneHead, "feat/one") + requiresCheck(w, p) + w.h.PullUpdated(p) + w.h.CommitStatus(checkStatus(oneRepo, oneHead, "success", oneTrunk)) + w.settleAll() + if w.op.count() != 0 { + t.Fatalf("the Approve was asked with no checker verdict at the head") + } + if d := w.delivery(IDOf(oneRepo, oneHead)); d.Ready == nil || !strings.Contains(d.Ready.Waits, "checker") { + t.Fatalf("it does not say it waits for the checker's verdict: %+v", d.Ready) + } + putCheckerVerdict(w, "running", headOf(oneRepo, 1, oneHead)) + w.settleAll() + if w.op.count() != 0 { + t.Fatalf("the Approve was asked on a running verdict") + } + putCheckerVerdict(w, "pass", headOf(oneRepo, 1, oneHead)) + w.settleAll() + if w.op.count() != 1 { + t.Fatalf("the Approve was asked %d times after the checker's pass, want once", w.op.count()) + } +} + +// 1302 acceptance (ADR 0447 §2, ADR 0441 §1): a group on mesh/check is asked once when every member's mesh/check passed +// and the checker's verdict names every member's head; a member's mesh/check that changes withdraws the ask. No composed +// check is asked of the controller and no member is owed a mesh/merge-gate. +func TestAGroupOnMeshCheckIsAskedOnTheCheckersOneVerdictAndWithdrawnWhenAMemberChanges(t *testing.T) { + w := newWorld(t) + w.op.auto = false + a, b := pr(oneRepo, 1, oneHead, "feat/two"), pr(oneRepo2, 2, oneHead2, "feat/two") + requiresCheck(w, a, b) + w.h.PullUpdated(a) + w.h.PullUpdated(b) + w.h.wantForge() // as the event's handler does for a head not seen before (main.go) + w.settleAll() + // One member's verdict alone, of a check that judged it by itself: not the group's. + putCheckerVerdict(w, "pass", headOf(oneRepo, 1, oneHead)) + w.h.CommitStatus(checkStatus(oneRepo, oneHead, "success", oneTrunk)) + w.h.CommitStatus(checkStatus(oneRepo2, oneHead2, "success", oneTrunk)) + w.settleAll() + if w.op.count() != 0 { + t.Fatalf("the group was asked on a verdict that does not name every member's head") + } + putCheckerVerdict(w, "pass", headOf(oneRepo, 1, oneHead), headOf(oneRepo2, 2, oneHead2)) + w.settleAll() + if w.op.count() != 1 || len(w.op.open()) != 1 { + t.Fatalf("the group was asked %d times (open %v), want once", w.op.count(), w.op.open()) + } + if len(w.ctl.checks) != 0 { + t.Fatalf("a check was asked of the controller: %v", w.ctl.checks) + } + if got := mergeGateOwed(w); len(got) != 0 { + t.Fatalf("mesh/merge-gate was owed: %v", got) + } + w.h.CommitStatus(checkStatus(oneRepo2, oneHead2, "pending", oneTrunk2)) + w.settleAll() + if len(w.op.open()) != 0 { + t.Fatalf("the group's ask stands though a member's mesh/check changed: %v", w.op.open()) + } +} + +// 1302: a failing mesh/check rejects the delivery; nothing is asked. +func TestAFailingMeshCheckRejects(t *testing.T) { + w := newWorld(t) + p := pr(oneRepo, 1, oneHead, "feat/one") + requiresCheck(w, p) + w.h.PullUpdated(p) + w.h.CommitStatus(checkStatus(oneRepo, oneHead, "failure", oneTrunk)) + w.settleAll() + if s := w.state(IDOf(oneRepo, oneHead)); s != Rejected { + t.Fatalf("it is %s, want rejected", s) + } + if w.op.count() != 0 || len(w.ctl.checks) != 0 { + t.Fatalf("asked: operator %d, controller %v", w.op.count(), w.ctl.checks) + } +} + +// 1302 (lead's (2)): the recheck verb refuses a delivery on a mesh/check base, saying the checker checks again on every +// move and where to read it. +func TestRecheckRefusesAMeshCheckDelivery(t *testing.T) { + w := newWorld(t) + p := pr(oneRepo, 1, oneHead, "feat/one") + requiresCheck(w, p) + w.h.PullUpdated(p) + w.h.CommitStatus(checkStatus(oneRepo, oneHead, "failure", oneTrunk)) + w.settleAll() + _, err := w.h.Recheck(IDOf(oneRepo, oneHead), "try again", "tester") + if err == nil || !strings.Contains(err.Error(), "checker_pull") || !strings.Contains(err.Error(), "checker") { + t.Fatalf("recheck answered %v, want a refusal naming the checker and checker_pull", err) + } + if len(w.ctl.checks) != 0 { + t.Fatalf("a check was asked of the controller: %v", w.ctl.checks) + } +} + +// 1302: the mesh/check line gives the head and the trunk it judged. +func TestTheCheckLineGivesHeadAndTrunk(t *testing.T) { + head, trunk := checkLine("pass " + oneHead[:12] + " on " + oneTrunk[:12] + " | unit 4 run, 0 kept | group g of 2 pull requests") + if head != oneHead[:12] || trunk != oneTrunk[:12] { + t.Fatalf("head %q trunk %q", head, trunk) + } + if head, trunk = checkLine("failed: a test"); head != "" || trunk != "" { + t.Fatalf("a line that names no commits gave %q %q", head, trunk) + } +} + +// 1302 with issue 1274's lost checks: a head on mesh/check never awaits a controller's check, so the build queue is not +// read for it and nothing is asked again. +func TestAHeadOnMeshCheckAwaitsNoControllersCheck(t *testing.T) { + at := time.Date(2026, 10, 22, 12, 0, 0, 0, time.UTC) + d := &Delivery{State: Proposed, Number: 1, CheckAsk: &CheckAsk{ID: "check-1", At: at}} + if _, ok := awaitedSince(d); !ok { + t.Fatal("the controller's asked check is not awaited") + } + d.Required = []string{ctxCheck} + if _, ok := awaitedSince(d); ok { + t.Fatal("a head on mesh/check awaits a controller's check") + } +} diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready.go index b68d1017..ca9c08b2 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready.go @@ -299,8 +299,8 @@ func (h *Holder) sayAfterLine(d *Delivery) func() { // second Approve. Nil when nothing needs asking. Under the lock; the ask runs outside it. func (h *Holder) askFresh(d *Delivery, now time.Time) func() { if d.State != Proposed || d.outOfDate() == "" || d.AskedAgain != nil || !d.Check.Passes() || d.Base != MergeBranch || - d.Number == 0 || d.TrunkNow == "" { - return nil + d.Number == 0 || d.TrunkNow == "" || d.judgedByCheck() { + return nil // a head on mesh/check is the checker's to judge again on its trunk's move (ADR 0447 §4) } // A group's member too (issue 1047, design 47: a move of the trunk asks every check again): the group's one Approve is // asked after its composed check, so a member ready before that has no Approve to carry; its check is asked at once @@ -383,6 +383,9 @@ func (r *ReadyRun) noteRefusal(d *Delivery, answer string) { // stays through a trunk move, ADR 0421 §3) and its check against the trunk's head is not on its way; one without an // Approve waits for that check first. func (h *Holder) readyWaits(d *Delivery) string { + if d.judgedByCheck() && d.State == Proposed && d.outOfDate() != "" && h.covering(d) == nil { + return "its mesh/check is " + d.outOfDate() + ": the checker checks it again against the trunk's head" + } switch { case d.State == Ready: case d.State == Proposed && d.outOfDate() != "" && d.AskedAgain != nil: @@ -402,6 +405,10 @@ func (h *Holder) readyWaits(d *Delivery) string { return w } } + // On mesh/check, the checker's verdict at exactly the heads the ask covers (issue 1302, ADR 0447 §2). + if w := h.checkerWaits(d); w != "" { + return w + } if d.Group == "" { return "" } diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/unchecked.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/unchecked.go index bbcbf204..77d4ed28 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/unchecked.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/unchecked.go @@ -54,9 +54,10 @@ const outOfDatePrefix = "out of date:" // minute and the check is asked at once, so ten minutes is a fault, never a queue. const UncheckedAfter = 10 * time.Minute -// mergeChecks are the statuses that are the mesh's merge check (novox/hq ADR 0237): a base requiring either is -// one the mesh is meant to check. -var mergeChecks = []string{"mesh/merge-gate", "mesh/repo-check"} +// mergeChecks are the statuses that are the mesh's check: mesh/check, the one main requires (novox/hq ADR 0447, issue +// 1302), and the two it replaced, which hq's base (mesh/repo-check, ADR 0503 §5) and a reverted one still require. A base +// requiring any is one the mesh is meant to check; only those a base requires are waited on. +var mergeChecks = []string{ctxCheck, "mesh/merge-gate", "mesh/repo-check"} // OpenPull is an open pull request as the forge says it (the gitea module's gitea_open_pulls): its head, its base, // the contexts the base's protection requires, the contexts its head has a status of, in any state, and each of @@ -179,6 +180,15 @@ func (h *Holder) SeeOpenPulls() error { now := h.Now() seen := map[string]unchecked{} for _, p := range open { + if slices.Contains(p.Required, ctxCheck) { + // What the forge says of a head on a mesh/check base is its verdict (issue 1302): kept, and taken. + if d := h.deliveries[IDOf(p.Owner+"/"+p.Repo, p.HeadSHA)]; d != nil { + h.noteRequired(d, p.Required) + if c := p.Checks[ctxCheck]; c.State != "" { + h.takeCheckStatus(p.Owner+"/"+p.Repo, p.HeadSHA, c.State, c.Description) + } + } + } waiting, pending, _ := p.unanswered() if len(waiting) == 0 { continue diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/verbs.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/verbs.go index 27e876c5..a4543b06 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/verbs.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/verbs.go @@ -152,8 +152,9 @@ type GroupLine struct { ID string `json:"id"` State string `json:"state"` Why string `json:"why"` - // ComposedGate says the group's gate is the composed check's: each member's own pull request check runs the - // repository's check alone, and the composed verdict sets its mesh/merge-gate (novox/hq ADR 0403 §1, issue 978). + // ComposedGate says the group's gate is the controller's composed check's: each member's own pull request check runs + // the repository's check alone, and the composed verdict sets its mesh/merge-gate (novox/hq ADR 0403 §1, issue 978). + // On mesh/check the group's check is the checker's one verdict at every member's head (ADR 0447 §2, issue 1302). ComposedGate bool `json:"composed_gate"` Check string `json:"composed,omitempty"` Members []string `json:"members"` @@ -215,8 +216,8 @@ type StalledLine struct { State State `json:"state"` // Walk is the walk a line of the state Unheld is about (novox/hq issue 362). Walk string `json:"walk,omitempty"` - // Waiting are the merge checks a line of the state Unanswered waits on, each as it reads — "mesh/repo-check - // never set", "mesh/merge-gate pending since " — so the controller's words name them without reading + // Waiting are the merge checks a line of the state Unanswered waits on, each as it reads — "mesh/check + // never set", "mesh/check pending since " — so the controller's words name them without reading // Says (novox/hq issue 438). Waiting []string `json:"waiting,omitempty"` // Checks are the same merge checks as data — context, state, since when — so the controller says their times in @@ -303,6 +304,10 @@ func (h *Holder) Recheck(id, why, by string) (string, error) { h.mu.Unlock() return "", fmt.Errorf("no delivery %q", id) } + if d.judgedByCheck() { + h.mu.Unlock() + return "", checkerRefusal(id) // the lead's decision for issue 1302: the checker serves no ask + } if d.State == Proposed { // A proposed delivery's check is asked again when the build queue holds none of its head (issue 1274). h.mu.Unlock() @@ -670,6 +675,10 @@ func (h *Holder) Close(id, why string) (string, error) { // does not take spends nothing. Called under the lock; it unlocks. func (h *Holder) reask(d *Delivery, why string) (string, error) { id := d.ID + if d.judgedByCheck() { + h.mu.Unlock() + return "", checkerRefusal(id) + } trial := *d trial.Transitions = nil if _, err := Apply(&trial, EvReask, true, Facts{Now: h.Now(), By: byH2, Why: why}); err != nil { diff --git a/modules/mesh-delivery-gate/module.json b/modules/mesh-delivery-gate/module.json index b9a06d0e..baf8d3b6 100644 --- a/modules/mesh-delivery-gate/module.json +++ b/modules/mesh-delivery-gate/module.json @@ -79,6 +79,7 @@ "mesh-delivery.groups", "mesh-delivery.pulls", "mesh-delivery-teller.answers", + "mesh-delivery-checker.verdicts", "mesh-controller.assignments", "node-engine.have" ],