From 67092161bda16c513c805170a8be65fa5cf4cd43 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 25 Oct 2026 18:20:58 +0100 Subject: [PATCH 1/3] Gate hands every member of an approved group, in pin order, and says a stopped pass (issue 1275) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Met again 2026-10-25: group fix/puller-wakes-on-capacity was merged half. Three causes in the gate: - The next member's turn compared the group's state with "delivering", a word GroupState no longer says since release 6 renamed it "merging" (ready.go groupTurnToMerge). Once the first member's merge was heard from the forge the group read merging, and the second member waited for ever. Group states are now named constants, the turn goes on in ready and merging (groupMayMerge), and a test holds every word to it. - Members merged in id order, so mesh-catalog went before the mesh-sdk commit it pins. membersOf now orders by mergeRank: mesh-sdk first, then members that change no go.mod or go.sum, then those that do. - No condition was raised: the controller's self-check that read `stalled` went with release 6 (mesh-controller dc456d56). The gate now says its stalled lines as conditions itself on the operator-channel seat (ADR 0437 §9) after every look, clears those listed no more, and keeps which it raised in its new bucket conditions. A held group's line says "half merged" when a member merged. A pass held for a required status no longer retries on a timer: the status turning success is the event that asks the merge again. --- modules/mesh-delivery-gate/README.md | 5 +- .../cmd/mesh-delivery-gate/conditions.go | 166 +++++++++++ .../cmd/mesh-delivery-gate/group_merge.go | 16 +- .../cmd/mesh-delivery-gate/group_test.go | 35 ++- .../cmd/mesh-delivery-gate/holder.go | 90 ++++-- .../cmd/mesh-delivery-gate/issue1275_test.go | 268 ++++++++++++++++-- .../cmd/mesh-delivery-gate/keeps.go | 2 +- .../cmd/mesh-delivery-gate/main.go | 3 + .../cmd/mesh-delivery-gate/manifest_test.go | 10 +- .../cmd/mesh-delivery-gate/react.go | 3 +- .../cmd/mesh-delivery-gate/ready.go | 17 +- .../cmd/mesh-delivery-gate/unchecked.go | 2 +- .../cmd/mesh-delivery-gate/verbs.go | 4 +- modules/mesh-delivery-gate/module.json | 6 +- 14 files changed, 563 insertions(+), 64 deletions(-) create mode 100644 modules/mesh-delivery-gate/cmd/mesh-delivery-gate/conditions.go diff --git a/modules/mesh-delivery-gate/README.md b/modules/mesh-delivery-gate/README.md index 32823a37..66299ef3 100644 --- a/modules/mesh-delivery-gate/README.md +++ b/modules/mesh-delivery-gate/README.md @@ -22,7 +22,10 @@ is gone is read as the state it came to (merged, closed or proposed) when the ga ## When a refused or rejected merge is said (ADR 0505 §4, §5) -`stalled` is what the controller raises as a condition for the operator. The gate's own list (`show`, `deliveries`) +`stalled` is what the gate says to the operator as conditions, itself, on the operator-channel seat (ADR 0437 §9, issue +1275): after every look it raises each line, raises it again when its severity or need changes, and clears each that is +listed no more; its bucket `conditions` keeps which it raised. (The controller's self-check that raised them went in +release 6.) The gate's own list (`show`, `deliveries`) names every refusal and rejection at once, and the pull request is told at once; only `stalled` waits: - a merge refused because its trunk moved, whose check the gate asked again, is stalled only once that check answered and diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/conditions.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/conditions.go new file mode 100644 index 00000000..84bb195e --- /dev/null +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/conditions.go @@ -0,0 +1,166 @@ +package main + +import ( + "encoding/json" + "fmt" + "sort" + "strings" + + stdio "git.novox.be/novox/mesh-sdk/go" + "git.novox.be/novox/mesh-sdk/go/asks" +) + +// The gate says its own stalled lines to the operator as conditions (novox/hq issue 1275, met again 2026-10-25; ADR 0437 +// §9: any module says a condition on the operator-channel seat's one subject, and the router alone chooses the channels). +// Until release 6 the controller's self-check read the `stalled` verb and raised each line as `delivery..stalled`; +// release 6 deleted that self-check (mesh-controller dc456d56) and nothing read the verb since, so a half-merged group was +// said to no one. Now, after every look, the gate raises each line `stalled` lists, raises it again when its severity or +// its need changes, and clears each it raised that is listed no more. What it raised is kept in its bucket conditions, so a +// gate that restarts clears what ended while it was away. + +// ConditionStore keeps the conditions the gate has raised: the condition's key to what it said (conditionSaid). +type ConditionStore interface { + Said() (map[string]string, error) + PutSaid(said map[string]string) error +} + +// saidKey is the one key of the bucket conditions. +const saidKey = "said" + +// kvConditions is the bucket conditions over the runtime. +type kvConditions struct{} + +func (kvConditions) Said() (map[string]string, error) { + e, err := stdio.State("conditions").Get(saidKey) + if err != nil || e == nil { + return map[string]string{}, err + } + said := map[string]string{} + if err := json.Unmarshal(e.Value, &said); err != nil { + return nil, fmt.Errorf("the conditions the gate raised are not readable: %w", err) + } + return said, nil +} + +func (kvConditions) PutSaid(said map[string]string) error { + _, err := stdio.State("conditions").Put(saidKey, said) + return err +} + +// sayOnSeat says a condition on the operator-channel seat under the gate's own name. +func sayOnSeat(c asks.Condition) error { + subject, err := asks.ConditionSubject("mesh-delivery-gate") + if err != nil { + return err + } + raw, err := asks.EncodeCondition(c) + if err != nil { + return err + } + return stdio.SeatPublish(subject, json.RawMessage(raw), "") +} + +// conditionKeyOf is the key of a stalled line's condition: its last part is the condition's kind. +func conditionKeyOf(id string) string { return "mesh-delivery-gate.delivery." + id + ".stalled" } + +// conditionSaid is what a raised condition said that, when it changes, raises it again: its severity and what it needs. +// The words of a line hold durations, which change at every look and are no reason to tell the operator again. +func conditionSaid(l StalledLine) string { return severityOfLine(l) + "|" + l.H2 + "|" + l.Needs } + +func severityOfLine(l StalledLine) string { + if l.Severity == asks.SeverityUrgent { + return asks.SeverityUrgent + } + return asks.SeverityWarning +} + +// conditionOf is a stalled line as the condition the operator is told. +func conditionOf(l StalledLine) asks.Condition { + headline := "stalled: " + l.ID + if strings.HasPrefix(l.Says, "group ") { + group, _, _ := strings.Cut(strings.TrimPrefix(l.Says, "group "), " ") + headline = "group " + group + " is not merged whole" + } + if r := []rune(headline); len(r) > asks.HeadlineLength { + headline = string(r[:asks.HeadlineLength-1]) + "…" + } + needs := "read mesh-delivery.show " + l.ID + " and decide" + return asks.Condition{Key: conditionKeyOf(l.ID), Severity: severityOfLine(l), State: asks.StateRaised, + Subject: asks.Scope{Scope: "delivery", ID: l.ID}, Summary: l.Says, Headline: headline, Needs: needs, + Explanation: asks.NeedsYou + needs + ". " + l.Says + " What ends it: " + l.H2 + ".", + Resolved: "The gate lists " + l.ID + " as stalled no more."} +} + +// sayConditions raises and clears the conditions of the stalled lines (see above). Outside the lock; React runs it after +// the look, so it follows every event. A condition that could not be said is said again at the next look, after +// askAgainAfter at the latest. +func (h *Holder) sayConditions() { + if h.Say == nil || h.Conditions == nil { + return + } + if h.said == nil { + said, err := h.Conditions.Said() + if err != nil { + h.Logf("[mesh-delivery] the conditions raised before cannot be read, so none is said yet: %v; asked again in %s", err, askAgainAfter) + h.again("the conditions the gate raised, read") + return + } + h.said = said + } + lines := map[string]StalledLine{} + for _, l := range h.Stalled() { + // One condition for an id: the most severe of its lines. + if o, ok := lines[l.ID]; !ok || (severityOfLine(o) != asks.SeverityUrgent && severityOfLine(l) == asks.SeverityUrgent) { + lines[l.ID] = l + } + } + ids := make([]string, 0, len(lines)) + for id := range lines { + ids = append(ids, id) + } + sort.Strings(ids) + changed, failed := false, false + for _, id := range ids { + l, key := lines[id], conditionKeyOf(id) + said := conditionSaid(l) + if h.said[key] == said { + continue + } + if err := h.Say(conditionOf(l)); err != nil { + h.Logf("[mesh-delivery] the condition of %s could not be said: %v", id, err) + failed = true + continue + } + h.Logf("[mesh-delivery] %s is said to the operator as %s: %s", id, severityOfLine(l), l.Says) + h.said[key], changed = said, true + } + var gone []string + for key := range h.said { + id := strings.TrimSuffix(strings.TrimPrefix(key, "mesh-delivery-gate.delivery."), ".stalled") + if _, ok := lines[id]; !ok { + gone = append(gone, key) + } + } + sort.Strings(gone) + for _, key := range gone { + id := strings.TrimSuffix(strings.TrimPrefix(key, "mesh-delivery-gate.delivery."), ".stalled") + c := asks.Condition{Key: key, Subject: asks.Scope{Scope: "delivery", ID: id}, State: asks.StateCleared, + Resolved: "The gate lists " + id + " as stalled no more."} + if err := h.Say(c); err != nil { + h.Logf("[mesh-delivery] the condition of %s could not be cleared: %v", id, err) + failed = true + continue + } + h.Logf("[mesh-delivery] the condition of %s is cleared: it is stalled no more", id) + delete(h.said, key) + changed = true + } + if changed { + if err := h.Conditions.PutSaid(h.said); err != nil { + h.Logf("[mesh-delivery] the conditions raised cannot be kept: %v", err) + } + } + if failed { + h.again("a condition that could not be said to the operator") + } +} diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/group_merge.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/group_merge.go index 4cfb1b6b..bb41b294 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/group_merge.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/group_merge.go @@ -63,7 +63,8 @@ func (h *Holder) unmergedLive(g *Group) []*Delivery { // groupMergeable reads every member's required statuses at the head the Approve covers, and says whether all are a success. // When one is not, or cannot be read, nothing is handed to the bridge: the group is held, said urgent (ADR 0441 §2, §4), and -// the target's turn is given back so the next event that decides whether it may merge asks it again. Outside the lock. +// the target's turn is given back so the next event that decides whether it may merge asks it again. A read that could not +// be made is a failed ask, tried again after askAgainAfter. Outside the lock. func (h *Holder) groupMergeable(gid, target string, reads []memberRead) bool { release := func() { if d := h.deliveries[target]; d != nil && d.Ready != nil { @@ -98,8 +99,9 @@ func (h *Holder) groupMergeable(gid, target string, reads []memberRead) bool { g.Hold = &GroupHold{Kind: "blocked", Member: m.id, Says: says, Since: since} h.keepGroup(g) } + // No timer: the status turning success is an event on the forge (CommitStatus, mergeAgainNow), and it asks the + // merge again at once (issue 1275: a pass that stops part way resumes on an event). release() - h.again("a group's merge held for a required status") h.mu.Unlock() return false } @@ -112,7 +114,7 @@ func (h *Holder) groupMergeable(gid, target string, reads []memberRead) bool { return true } -// groupLines are the groups that are held or half merged, as urgent stalled lines the controller raises as conditions: +// groupLines are the groups that are held or half merged, as urgent stalled lines the gate says as conditions (conditions.go): // one for each, on its first member not merged, until every member is merged or a person decides (issue 1275). A member // the merge tool refused is said by readyLines, with its group; this adds a group held for a required status, and a // group half merged with no refusal. Under the lock. @@ -141,12 +143,16 @@ func (h *Holder) groupLines(now time.Time) []StalledLine { switch { // A held member the merge tool refused is said by readyLines already: one line for an id, not two. case g.Hold != nil && g.Hold.Kind == "blocked" && !running && !(heldOn != nil && heldOn.Ready != nil && heldOn.Ready.RefusedSince != nil): + stands := "is approved and nothing is merged" + if merged > 0 { + stands = "is half merged" // a member merged before the pass stopped (issue 1275) + } out = append(out, StalledLine{ID: g.Hold.Member, Number: numberOf(h.deliveries[g.Hold.Member]), State: first.State, For: now.Sub(g.Hold.Since).Round(time.Second).String(), Bound: "0s", Severity: "urgent", H2: "none: the required status turns success, which asks the merge again — or the operator's decision", - Says: fmt.Sprintf("group %s is approved and nothing is merged (merged %d of %d): %s waits on a required status "+ + Says: fmt.Sprintf("group %s %s (merged %d of %d): %s waits on a required status "+ "at its head: %s. Needed: that status turns success (its check asked again), or the operator decides.", - g.ID, merged, len(h.membersOf(g)), g.Hold.Member, g.Hold.Says)}) + g.ID, stands, merged, len(h.membersOf(g)), g.Hold.Member, g.Hold.Says)}) case merged > 0 && !refused && !running: out = append(out, StalledLine{ID: first.ID, Number: first.Number, State: first.State, For: now.Sub(g.Updated).Round(time.Second).String(), Bound: "0s", Severity: "urgent", diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/group_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/group_test.go index e37154a7..9e5ac63f 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/group_test.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/group_test.go @@ -21,25 +21,44 @@ const ( // approvedGroup is a group of two pull requests on one branch, checks green, composed green and the operator's Approve // asked and not yet answered (w.op.auto is off). func approvedGroup(t *testing.T) *world { + return approvedGroupOf(t, member{gCtl, 1, gHc, "internal/a.go"}, member{gCat, 2, gHk, "modules/app/module.json"}) +} + +// member is a pull request of a group in feat/g: its repository, number, head and the paths it changes. +type member struct { + repo string + number int + head string + paths string +} + +// approvedGroupOf is a group feat/g of these members, each checked, composed together and asked once. +func approvedGroupOf(t *testing.T, ms ...member) *world { + t.Helper() w := newWorld(t) w.op.auto = false - w.h.PullUpdated(pr(gCtl, 1, gHc, "feat/g", "internal/a.go")) - w.h.PullUpdated(pr(gCat, 2, gHk, "feat/g", "modules/app/module.json")) - w.h.Checked(judged(gCtl, 1, gHc, "pass", gTrunk0)) - w.h.Checked(judged(gCat, 2, gHk, "pass", gTrunk0)) + for _, m := range ms { + w.h.PullUpdated(pr(m.repo, m.number, m.head, "feat/g", strings.Split(m.paths, ",")...)) + } + for _, m := range ms { + w.h.Checked(judged(m.repo, m.number, m.head, "pass", gTrunk0)) + } w.settleAll() g := w.h.groups["feat/g"] if g == nil || g.Check == nil { t.Fatalf("no composed check was asked: %+v", g) } - w.h.Checked(CheckedEvent{Group: "feat/g", ID: g.Check.Asked, Verdict: "pass", Summary: "composed", - Members: []struct { + c := CheckedEvent{Group: "feat/g", ID: g.Check.Asked, Verdict: "pass", Summary: "composed"} + for _, m := range ms { + owner, repo, _ := strings.Cut(m.repo, "/") + c.Members = append(c.Members, struct { Owner string `json:"owner"` Repo string `json:"repo"` Commit string `json:"commit"` Trunk string `json:"trunk,omitempty"` - }{{Owner: "novox", Repo: "mesh-controller", Commit: gHc, Trunk: gTrunk0[:12]}, - {Owner: "novox", Repo: "mesh-catalog", Commit: gHk, Trunk: gTrunk0[:12]}}}) + }{Owner: owner, Repo: repo, Commit: m.head, Trunk: gTrunk0[:12]}) + } + w.h.Checked(c) w.settleAll() if w.op.count() != 1 || len(w.op.open()) != 1 { t.Fatalf("the green group is asked once: %d asks, open %v", w.op.count(), w.op.open()) 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 ecbf057f..851305e0 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go @@ -10,6 +10,8 @@ import ( "strings" "sync" "time" + + "git.novox.be/novox/mesh-sdk/go/asks" ) // The holder of the mesh-delivery seat (novox/hq ADR 0239, to-be 47): the one writer of deliveries and @@ -37,6 +39,11 @@ type Holder struct { Owed OwedStore // Pulls keeps the pull requests' keys on the bus (pullkey.go); nil keeps none. Pulls PullKeys + // Say says a condition to the operator, and Conditions keeps which the gate raised (conditions.go); nil says none. + Say func(c asks.Condition) error + Conditions ConditionStore + // said is what Conditions keeps, read at the first look; touched by React alone. + said map[string]string // cancels are asks to take back, gathered under the lock and run with the other asks of the look. cancels []func() @@ -921,7 +928,7 @@ func (h *Holder) askChecks(now time.Time) func() { } } -// membersOf are a group's current members, by id order. +// membersOf are a group's current members, in the order they merge (mergeRank, then id). func (h *Holder) membersOf(g *Group) []*Delivery { var out []*Delivery for _, id := range g.Members { @@ -929,9 +936,39 @@ func (h *Holder) membersOf(g *Group) []*Delivery { out = append(out, d) } } + sort.SliceStable(out, func(i, j int) bool { + if ri, rj := mergeRank(out[i]), mergeRank(out[j]); ri != rj { + return ri < rj + } + return out[i].ID < out[j].ID + }) return out } +// SDKRepository is the repository of mesh-sdk, the library the other repositories' modules pin by commit (novox/hq ADR 0482 §3). +const SDKRepository = "novox/mesh-sdk" + +// mergeRank is a member's place in its group's merge (novox/hq issue 1275): a member another member pins merges before it, so +// that the trunk never holds a pin of a commit not on its own trunk. The gate knows a pin by the paths a pull request changes: +// mesh-sdk's member goes first (the members of the other repositories pin it, ADR 0482 §3); then the members that change no +// go.mod or go.sum (they pin nothing new); then those that do (they pin another member's commit). The id orders a tie. The +// module graph is the fuller source (which module requires which), and is not the gate's to read: this is what its own +// deliveries say. +func mergeRank(d *Delivery) int { + if strings.EqualFold(d.Repository, SDKRepository) { + return 0 + } + if d.PathsTruncated { + return 2 // its paths are not all known: it may pin, so it goes after those that do not + } + for _, p := range d.Paths { + if base := p[strings.LastIndex(p, "/")+1:]; base == "go.mod" || base == "go.sum" { + return 2 + } + } + return 1 +} + // planGroup is what a group needs asked, and its turns. func (h *Holder) planGroup(g *Group, members []*Delivery, now time.Time) []func() { var asks []func() @@ -956,7 +993,7 @@ func (h *Holder) planGroup(g *Group, members []*Delivery, now time.Time) []func( if held { status = "pending" // no Approve exists while a member waits for its review (novox/hq ADR 0496 §2) } - if state == "merging" && !(g.Check != nil && (g.Check.Verdict == "pass" || g.Check.Verdict == "warning") && + if state == GroupMerging && !(g.Check != nil && (g.Check.Verdict == "pass" || g.Check.Verdict == "warning") && sameMembers(g.Check.Commits, commitsOf(members)) && g.Check.OutOfDate == "") { // Success there lets the merge tool merge its members: only for a composed check that passed for these heads. status = "pending" @@ -980,6 +1017,23 @@ func (h *Holder) planGroup(g *Group, members []*Delivery, now time.Time) []func( return asks } +// The words GroupState says a group's state with (ADR 0539 §17). Code that decides on a group's state compares with these, +// never with a literal: a word GroupState no longer says ("delivering", retired in release 6) left the second member of +// every approved group waiting for ever (novox/hq issue 1275, met again 2026-10-25). groupWords lists them all, and a test +// holds every decision on a group's state to that list. +const ( + GroupChecking = "checking" + GroupRejected = "rejected" + GroupReady = "ready" + GroupMerging = "merging" + GroupMerged = "merged" + GroupHeld = "held" + GroupClosed = "closed" +) + +// groupWords are every word GroupState says. +var groupWords = []string{GroupChecking, GroupRejected, GroupReady, GroupMerging, GroupMerged, GroupHeld, GroupClosed} + // GroupState is a group's state, derived from its members and its composed check — never set apart (novox/hq // ADR 0239 decision 4, cut to the merge's states by ADR 0539 §17): checking, rejected, ready, merging (some members // merged), merged, held and closed. Nothing after the merge is derived here: whether a merged change reached the machines is @@ -992,36 +1046,36 @@ func GroupState(g *Group, members []*Delivery) (string, string) { n := len(members) switch { case count[Held] > 0 && g.Closed: - return "held", "a member is held: " + heldWords(members) + return GroupHeld, "a member is held: " + heldWords(members) case count[Closed] > 0 && g.Closed: - return "closed", "a member was closed before the group merged" + return GroupClosed, "a member was closed before the group merged" case n > 0 && count[Merged] == n: - return "merged", "every member is merged" + return GroupMerged, "every member is merged" case g.Closed && g.Check != nil && (g.Check.OutOfDate != "" || g.Check.Verdict == "" || !sameMembers(g.Check.Commits, commitsOf(members))): // A member rebased or pushed once the group was merging, or its composed check out of date: it is composed // again for these heads, and the group is checking until then, never held (novox/hq ADR 0323 §6, issue 406). if g.Check.OutOfDate != "" { - return "checking", g.Check.OutOfDate + return GroupChecking, g.Check.OutOfDate } - return "checking", "composed again for its members' new heads" + return GroupChecking, "composed again for its members' new heads" case g.Closed && g.Check != nil && g.Check.Verdict != "pass" && g.Check.Verdict != "warning": // Composed again for new heads once it was merging, and those do not compose: its members wait (issue 902). - return "rejected", "composed together: " + g.Check.Summary + return GroupRejected, "composed together: " + g.Check.Summary case g.Closed: - return "merging", fmt.Sprintf("%d of %d merged", count[Merged], n) + return GroupMerging, fmt.Sprintf("%d of %d merged", count[Merged], n) case count[Rejected] > 0: - return "rejected", "a member's own check failed" + return GroupRejected, "a member's own check failed" case g.Check != nil && g.Check.OutOfDate != "": // Out of date counts as outstanding (ADR 0239 §18, novox/hq ADR 0323 §7): checking, saying why. - return "checking", g.Check.OutOfDate + return GroupChecking, g.Check.OutOfDate case g.Check != nil && g.Check.Verdict != "" && g.Check.Verdict != "pass" && g.Check.Verdict != "warning": - return "rejected", "composed together: " + g.Check.Summary + return GroupRejected, "composed together: " + g.Check.Summary case count[Ready] == n && g.Check != nil && sameMembers(g.Check.Commits, commitsOf(members)) && (g.Check.Verdict == "pass" || g.Check.Verdict == "warning"): - return "ready", "every member ready, and composed together they pass" + return GroupReady, "every member ready, and composed together they pass" } - return "checking", fmt.Sprintf("%d of %d member(s) ready", count[Ready], n) + return GroupChecking, fmt.Sprintf("%d of %d member(s) ready", count[Ready], n) } // heldWords says the held members and why. @@ -1040,7 +1094,7 @@ func heldWords(members []*Delivery) string { // says its mesh/delivery-group is pending, not a success. Under the lock. func (h *Holder) groupState(g *Group, members []*Delivery) (state, why string, held bool) { state, why = GroupState(g, members) - if state != "ready" || g.Closed { + if state != GroupReady || g.Closed { return state, why, false } var waiting []string @@ -1063,11 +1117,11 @@ func (h *Holder) groupState(g *Group, members []*Delivery) (state, why string, h // heads (novox/hq ADR 0382), and a pending one there would hold them for ever (novox/hq issue 902). func groupStatusState(s string) string { switch s { - case "ready", "merging", "merged": + case GroupReady, GroupMerging, GroupMerged: return "success" - case "rejected": + case GroupRejected: return "failure" - case "held", "closed": + case GroupHeld, GroupClosed: return "error" } return "pending" diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1275_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1275_test.go index 61c318e3..451ea831 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1275_test.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1275_test.go @@ -3,8 +3,12 @@ package main import ( "errors" "strings" + "sync" "testing" "time" + + "git.novox.be/novox/mesh-sdk/go/asks" + sdk "git.novox.be/novox/mesh-sdk/go/delivery" ) const time30s = 31 * time.Second @@ -75,24 +79,6 @@ func TestAnApprovedGroupWithAMemberWhoseRequiredStatusIsNotASuccessMergesNothing } } -// 1275/AC3: the merge tool refuses the first member in order; the second is not merged, and the group is said urgent, -// naming the group, the refused member and the forge's words. -func TestARefusedFirstMemberStopsTheGroupAndIsSaidUrgent(t *testing.T) { - w := approvedGroup(t) - allGreen(w) - w.forge.refuseRepo = map[string]error{gCat: refusedWords("novox/mesh-catalog#2")} - w.op.answer(w.op.open()[0], OptionApprove) - w.settleAll() - if len(w.forge.merges) != 1 || !strings.Contains(w.forge.merges[0], "mesh-catalog#2") { - t.Fatalf("wanted only the first member asked: %v", w.forge.merges) - } - lines := urgentLines(w) - if len(lines) != 1 || lines[0].ID != IDOf(gCat, gHk) || !strings.Contains(lines[0].Says, "feat/g") || - !strings.Contains(lines[0].Says, "mesh/repo-check (missing)") || !strings.Contains(lines[0].Says, "Needed") { - t.Fatalf("not said as one urgent line: %+v", lines) - } -} - // 1275/AC4: a group half merged (the second member refused after the first merged) stays raised, pass after pass, until // the second is merged; then it goes. func TestAHalfMergedGroupStaysRaisedUntilEveryMemberIsMerged(t *testing.T) { @@ -344,3 +330,249 @@ func TestASeverityOnceUrgentNeverStepsDownToAWarning(t *testing.T) { t.Fatalf("severity %v after an out of date refusal that followed an urgent one; wanted urgent", got) } } + +// fakeConditions is the gate's bucket conditions and the operator-channel seat's subject for conditions: what it keeps, +// and every condition said, in order. +type fakeConditions struct { + mu sync.Mutex + kept map[string]string + told []asks.Condition +} + +func (f *fakeConditions) Said() (map[string]string, error) { + f.mu.Lock() + defer f.mu.Unlock() + out := map[string]string{} + for k, v := range f.kept { + out[k] = v + } + return out, nil +} + +func (f *fakeConditions) PutSaid(said map[string]string) error { + f.mu.Lock() + defer f.mu.Unlock() + f.kept = map[string]string{} + for k, v := range said { + f.kept[k] = v + } + return nil +} + +func (f *fakeConditions) say(c asks.Condition) error { + if _, err := asks.EncodeCondition(c); err != nil { + return err // what the router would refuse + } + f.mu.Lock() + defer f.mu.Unlock() + f.told = append(f.told, c) + return nil +} + +// last is the newest condition said for a delivery id, or nil. +func (f *fakeConditions) last(id string) *asks.Condition { + f.mu.Lock() + defer f.mu.Unlock() + for i := len(f.told) - 1; i >= 0; i-- { + if f.told[i].Subject.ID == id { + c := f.told[i] + return &c + } + } + return nil +} + +// hearConditions has the gate say its conditions into a fake seat and bucket. +func hearConditions(w *world) *fakeConditions { + f := &fakeConditions{kept: map[string]string{}} + w.h.Say, w.h.Conditions = f.say, f + return f +} + +// handedInOrder answers each member the gate hands the forge bridge as the forge does on 2026-10-25: the forge's word that +// it merged (gitea.pull.merged) first, the bridge's word after; or the bridge's first, as forgeFirst says. It returns the +// repositories in the order they were handed. +func handedInOrder(w *world, forgeFirst bool) []string { + var handed []string + numbers := map[string]int{} + for i := 0; i < 4; i++ { + var next *sdk.MergePull + for _, p := range w.merges.last("feat/g").Pulls { + if p.State == sdk.MergeMerging { + p := p + next = &p + } + } + if next == nil || numbers[next.Repository] > 0 { + break + } + handed = append(handed, next.Repository) + numbers[next.Repository] = next.Number + mergeCommit := strings.Repeat(string(rune('a'+i)), 40) + forge := func() { w.h.PullMerged(merged(next.Repository, next.Number, next.Head.Head, mergeCommit)) } + bridge := func() { bridgeMerged(w, next.Repository, next.Number, next.Head.Head, mergeCommit) } + if forgeFirst { + forge() + w.settleAll() // ten seconds pass between the two words on 2026-10-25: the gate looks in between + bridge() + } else { + bridge() + w.settleAll() + forge() + } + w.settleAll() + } + return handed +} + +// Issue 1275, met again 2026-10-25 (group fix/puller-wakes-on-capacity): every member of an approved group is handed to +// the forge bridge, in the group's order, the next on the event that the one before merged; a group whose pass stops is +// said to the operator as a condition, which is cleared once it is whole; and every word a group's state is said in is +// one the merge decides on. +func TestAnApprovedGroupIsHandedWholeInItsOrderAndAStopIsSaid(t *testing.T) { + const ( + sdkRepo, hS = "novox/mesh-sdk", "a8fbabbfc37369f9d1f6977c72e5523aff3d494c" + catRepo, hK = "novox/mesh-catalog", "23c827e7910c233608af4e4a8056cbc362dcb755" + ctlRepo, hC = "novox/mesh-controller", "c0c0c0c0c0c0c0c0c0c0c0c0c0c0c0c0c0c0c0c0" + ) + sdkMember := member{sdkRepo, 80, hS, "go/stdio.go,go/buildstep/pull.go"} + catMember := member{catRepo, 479, hK, "modules/mesh-build-go/asks.go,modules/mesh-build-go/go.mod,modules/mesh-build-go/go.sum"} + ctlMember := member{ctlRepo, 7, hC, "cmd/mesh-controller/main.go"} + inOrder := func(members []member, forgeFirst bool, want ...string) func(t *testing.T) { + return func(t *testing.T) { + w := approvedGroupOf(t, members...) + w.merges.manual = true + allGreenFor(w, members...) + w.op.answer(w.op.open()[0], OptionApprove) + w.settleAll() + if got := handedInOrder(w, forgeFirst); strings.Join(got, " ") != strings.Join(want, " ") { + t.Fatalf("handed %v, wanted %v, every member in the group's order", got, want) + } + if g := w.h.groups["feat/g"]; g == nil { + t.Fatal("the group is gone") + } else if state, why := GroupState(g, w.h.membersOf(g)); state != GroupMerged { + t.Fatalf("the group is %s (%s), not merged whole", state, why) + } + if w.source.main == nil || len(w.source.main.Heads) != len(want) { + t.Fatalf("source.main does not hold the group as one revision: %+v", w.source.main) + } + } + } + cases := []struct { + name string + run func(t *testing.T) + }{ + // 1275/AC3 (2026-10-25): the case of the day. The catalogue member pins the sdk member's commit; the forge's word + // of a merge arrives before the bridge's. Before the fix the catalogue member went first, and the sdk member was + // never handed: its group read merging, a word the turn to merge did not know. + {"sdk first, then the member that pins it, the forge's word first", inOrder([]member{catMember, sdkMember}, true, sdkRepo, catRepo)}, + {"sdk first, then the member that pins it, the bridge's word first", inOrder([]member{sdkMember, catMember}, false, sdkRepo, catRepo)}, + {"a member that pins nothing before one that changes a go.mod", inOrder([]member{catMember, ctlMember}, true, ctlRepo, catRepo)}, + {"three members", inOrder([]member{ctlMember, catMember, sdkMember}, true, sdkRepo, ctlRepo, catRepo)}, + // 1275/AC3: the merge tool refuses the first member in order; the second is not merged, and the group is said + // urgent, naming the group, the refused member and the forge's words, as a stalled line and to the operator. + {"a refused first member stops the group and is said urgent", func(t *testing.T) { + w := approvedGroup(t) + told := hearConditions(w) + allGreen(w) + w.forge.refuseRepo = map[string]error{gCat: refusedWords("novox/mesh-catalog#2")} + w.op.answer(w.op.open()[0], OptionApprove) + w.settleAll() + if len(w.forge.merges) != 1 || !strings.Contains(w.forge.merges[0], "mesh-catalog#2") { + t.Fatalf("wanted only the first member asked: %v", w.forge.merges) + } + lines := urgentLines(w) + if len(lines) != 1 || lines[0].ID != IDOf(gCat, gHk) || !strings.Contains(lines[0].Says, "feat/g") || + !strings.Contains(lines[0].Says, "mesh/repo-check (missing)") || !strings.Contains(lines[0].Says, "Needed") { + t.Fatalf("not said as one urgent line: %+v", lines) + } + if c := told.last(IDOf(gCat, gHk)); c == nil || c.State != asks.StateRaised || c.Severity != asks.SeverityUrgent || + !strings.Contains(c.Summary, "feat/g") { + t.Fatalf("the operator was not told, urgent: %+v", c) + } + }}, + // 1275/AC4: a group half merged (its pass stopped after the first member) raises one urgent condition, said once + // however many looks follow, and cleared once the group is whole. + {"a half-merged group is raised once and cleared once whole", func(t *testing.T) { + w := approvedGroup(t) + told := hearConditions(w) + allGreen(w) + w.merges.manual = true + w.op.answer(w.op.open()[0], OptionApprove) + w.settleAll() + w.h.PullMerged(merged(gCat, 2, gHk, gT1)) + bridgeMerged(w, gCat, 2, gHk, gT1) + setChecks(w, gCtl, gHc, "success", "") // the second member's status went missing: the pass stops at it + w.settleAll() + for range 3 { + w.later(time30s) + w.settleAll() + } + id := IDOf(gCtl, gHc) + raised := 0 + for _, c := range told.told { + if c.Subject.ID == id && c.State == asks.StateRaised { + raised++ + } + } + if c := told.last(id); raised != 1 || c == nil || c.Severity != asks.SeverityUrgent || !strings.Contains(c.Summary, "feat/g") || + !strings.Contains(c.Summary, "half merged (merged 1 of 2)") { + t.Fatalf("raised %d times, last %+v; wanted once, urgent, naming the group half merged", raised, c) + } + setChecks(w, gCtl, gHc, "success", "success") + w.h.CommitStatus(ForgeStatusEvent{Owner: "novox", Repo: "mesh-controller", SHA: gHc, Context: repoCheck, State: "success"}) + w.settleAll() // the status is an event: the pass goes on with no timer + handed := false + for _, p := range w.merges.last("feat/g").Pulls { + handed = handed || (p.Repository == gCtl && p.State == sdk.MergeMerging) + } + if !handed { + t.Fatalf("the status turned success and the second member was not handed: %+v", w.merges.last("feat/g")) + } + w.h.PullMerged(merged(gCtl, 1, gHc, gT1)) + bridgeMerged(w, gCtl, 1, gHc, gT1) + w.settleAll() + if c := told.last(id); c == nil || c.State != asks.StateCleared { + t.Fatalf("the group is whole and its condition is not cleared: %+v", c) + } + }}, + // A condition the gate raised before it restarted, whose delivery stalls no more, is cleared at the first look. + {"a condition kept from before a restart is cleared", func(t *testing.T) { + w := newWorld(t) + told := hearConditions(w) + told.kept[conditionKeyOf("novox/app@0123456789ab")] = "urgent||" + w.settleAll() + if c := told.last("novox/app@0123456789ab"); c == nil || c.State != asks.StateCleared || len(told.kept) != 0 { + t.Fatalf("not cleared: %+v, kept %v", c, told.kept) + } + }}, + // The kind behind the day's fault: a decision on a group's state named a word GroupState no longer says. Every word + // it says is one groupStatusState knows, and the merge goes on exactly in ready and merging. + {"every word of a group's state is decided on", func(t *testing.T) { + for _, word := range groupWords { + if groupStatusState(word) == "pending" && word != GroupChecking { + t.Errorf("groupStatusState does not know %q", word) + } + if may := groupMayMerge(word); may != (word == GroupReady || word == GroupMerging) { + t.Errorf("groupMayMerge(%q) = %v", word, may) + } + } + }}, + } + for _, c := range cases { + t.Run(c.name, c.run) + } +} + +// allGreenFor makes both merge checks a success at every member's head. +func allGreenFor(w *world, ms ...member) { + for _, m := range ms { + setChecks(w, m.repo, m.head, "success", "success") + } +} + +// bridgeMerged is the forge bridge's word that it merged a member. +func bridgeMerged(w *world, repository string, number int, head, merge string) { + w.merges.say(sdk.Forge{Kind: sdk.ForgeMerged, Repository: repository, Number: number, Head: head, Merge: merge, Root: "root", + Trees: map[string]string{"app": "tree"}}) +} diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/keeps.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/keeps.go index bf93b7db..0bd8150c 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/keeps.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/keeps.go @@ -6,7 +6,7 @@ import "git.novox.be/novox/mesh-sdk/go/announce" // issue 1790): the module says it itself, from its code, before it serves, and a reader is granted what it said // (clause 3). A state is readable when another module's manifest reads it (keeps_test.go names the readers). keeps_test.go holds the list to the // manifest's `state`, and keeps_integration_test.go that the module announces it on a bus. -var Kept = []announce.State{{Name: "asks", Readable: true}, {Name: "deliveries"}, {Name: "groups"}, {Name: "merges", Readable: true}, {Name: "owed", Readable: true}, {Name: "pulls", Readable: true}, {Name: "source", Readable: true}} +var Kept = []announce.State{{Name: "asks", Readable: true}, {Name: "conditions"}, {Name: "deliveries"}, {Name: "groups"}, {Name: "merges", Readable: true}, {Name: "owed", Readable: true}, {Name: "pulls", Readable: true}, {Name: "source", Readable: true}} // announceKept says Kept through keeps, which is stdio.Keeps. func announceKept(keeps func(...announce.State) error) error { return keeps(Kept...) } 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 264ef65c..072ee780 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/main.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/main.go @@ -67,6 +67,9 @@ func main() { Source: kvSource{}, Owed: kvOwed{}, Pulls: kvPulls{}, + // The gate says what stalled to the operator itself (issue 1275, ADR 0437 §9). + Say: sayOnSeat, + Conditions: kvConditions{}, } h.init() l := &listening{now: "not yet: starting"} 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 aea23e81..eb9d8a66 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 @@ -91,9 +91,9 @@ func TestItClaimsTheDeliverySeatAndFollowsWhatItHandles(t *testing.T) { if !reflect.DeepEqual(m.Emits, []string{"transition", "group", "merged"}) { t.Fatalf("emits %v", m.Emits) } - // Seven buckets, and no `asked` (the teller asks, ADR 0437 §6): ADR 0450 §3. + // Eight buckets, and no `asked` (the teller asks, ADR 0437 §6): ADR 0450 §3, and the conditions it raised (issue 1275). buckets := m.buckets() - if !reflect.DeepEqual(buckets, []string{"deliveries", "groups", "pulls", "asks", "merges", "owed", "source"}) { + if !reflect.DeepEqual(buckets, []string{"deliveries", "groups", "pulls", "asks", "merges", "owed", "source", "conditions"}) { t.Fatalf("state %v", buckets) } // What it reads: the teller's answers and the checker's verdicts (issue 1302). The carry from today's module's buckets, the @@ -102,10 +102,10 @@ func TestItClaimsTheDeliverySeatAndFollowsWhatItHandles(t *testing.T) { t.Fatalf("reads %v", m.Reads) } // It calls no controller verb and reads the forge with three tools; the forge's writes are the bridge's (ADR 0437 §8). It - // uses no operator-channel: the teller asks (ADR 0437 §6). + // uses operator-channel only to say its conditions (ADR 0437 §9, issue 1275): the teller asks (ADR 0437 §6). want := []string{"gitea.gitea_commit_statuses", "gitea.gitea_contains", "gitea.gitea_open_pulls"} - if len(m.Uses) != 0 { - t.Fatalf("uses %v: the teller asks, not the gate", m.Uses) + if !reflect.DeepEqual(m.Uses, []string{"operator-channel"}) { + t.Fatalf("uses %v: operator-channel, for its conditions alone", m.Uses) } if !reflect.DeepEqual(m.Invokes, want) { t.Fatalf("invokes %v, wanted %v", m.Invokes, want) diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/react.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/react.go index c843eca4..1e07a04c 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/react.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/react.go @@ -57,7 +57,7 @@ func (h *Holder) Work() { // React takes every step the facts allow, after an event that can change what they are: the forge's open pull requests // read when a check may have answered, the steps that need another's word (look), what is owed (Flush), and then -// the time the next look is wanted at, if any step waits on the clock. Never concurrent with itself. +// the conditions of what stalled (conditions.go), and then the time the next look is wanted at, if any step waits on the clock. Never concurrent with itself. func (h *Holder) React() { h.reactMu.Lock() defer h.reactMu.Unlock() @@ -75,6 +75,7 @@ func (h *Holder) React() { } h.look() h.Flush() + h.sayConditions() h.arm() } 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 774843ea..0d600ef8 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready.go @@ -29,7 +29,7 @@ import ( // - a group member only once the group's composed check passed for its members' heads as they are; // - a push ends the Approve and says so once on the pull request (ADR 0421 §6); the new head is asked on its own; // - a check that failed merges nothing: it is said once on the pull request, and as a stalled line, which the -// controller raises as the condition `delivery..stalled`. So is a merge the tool refused for an hour. +// gate says to the operator as a condition (conditions.go). So is a merge the tool refused for an hour. // // The review record is read and put in the ask; it is never a gate (ADR 0421 §5). What this owner does is kept on the // delivery (Ready), said in its log, and the merge's transitions name the Approve as their warrant. @@ -401,13 +401,14 @@ func (d *Delivery) holdsForReview() bool { // groupTurnToMerge is why a group member may not merge yet; empty when it may: the group's composed check passed for // its members' heads as they are and is not out of date. No member waits for another's merge or delivery (novox/hq // ADR 0418 §4): once the composed check passed and the Approve covers the heads, every member is merged in one pass. +// A group part way through that pass is merging, and its next member's turn is then (issue 1275). func (h *Holder) groupTurnToMerge(d *Delivery) string { g := h.groups[d.Group] if g == nil { return "" } members := h.membersOf(g) - if state, why := GroupState(g, members); state != "ready" && state != "delivering" { + if state, why := GroupState(g, members); !groupMayMerge(state) { return fmt.Sprintf("its group %s is %s: %s", g.ID, state, why) } if c := g.Check; c == nil || (c.Verdict != "pass" && c.Verdict != "warning") || c.OutOfDate != "" || @@ -417,6 +418,16 @@ func (h *Holder) groupTurnToMerge(d *Delivery) string { return "" } +// groupMayMerge is whether a group in this state hands its next member to the forge bridge: one ready, and one merging (a +// member merged, the pass goes on). Every other word holds the pass. +func groupMayMerge(state string) bool { + switch state { + case GroupReady, GroupMerging: + return true + } + return false +} + // sayOn owes a comment on a delivery's pull request: an owed act numbered among the comments on its head, written as a key // with the delivery's other effects (owed.go). Under the lock; what it returns has nothing left to do. func (h *Holder) sayOn(d *Delivery, body string) func() { @@ -427,7 +438,7 @@ func (h *Holder) sayOn(d *Delivery, body string) func() { } // readyLines are the approved heads that will not merge: a check that failed at once, a merge refused past its bound; -// as stalled lines, which the controller raises as conditions. Under the lock. +// as stalled lines, which the gate says as conditions (conditions.go). Under the lock. func (h *Holder) readyLines(now time.Time) []StalledLine { var out []StalledLine for _, d := range h.deliveries { 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 77d4ed28..2c9d92ba 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/unchecked.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/unchecked.go @@ -16,7 +16,7 @@ import ( // (novox/hq ADR 0406 §1, issue 975) — and once more at a head's bound, and one whose // base requires the merge check — mesh/merge-gate or mesh/repo-check — with neither on its head for UncheckedAfter // since this owner first saw it so is said as a delivery held: a `stalled` line in the state Unannounced, which the -// controller raises as `delivery..stalled` (D14) like any other delivery past its bound. The delivery domain's own +// gate says as a condition (conditions.go) like any other delivery past its bound. The delivery domain's own // word, so no second condition and no second judge: the id is the one its delivery will have once announced. // // A head whose check started and never answered is said the same way (novox/hq issue 438). Counting any merge-check 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 58e223f7..f1da969c 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/verbs.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/verbs.go @@ -129,7 +129,7 @@ func (h *Holder) Groups(all bool) []GroupLine { continue } state, why, _ := h.groupState(g, members) - if !all && (state == "merged" || state == "closed") { + if !all && (state == GroupMerged || state == GroupClosed) { continue } l := GroupLine{ID: g.ID, State: state, Why: why, ComposedGate: true} @@ -185,7 +185,7 @@ type StalledLine struct { // NeedsNewHead is what a stalled line needs when its cure is its author's new head (ADR 0505 §5). const NeedsNewHead = "new-head" -// Stalled is the `stalled` verb: what the controller's self-check raises as `delivery..stalled`. +// Stalled is the `stalled` verb: what the gate says to the operator as conditions (conditions.go). func (h *Holder) Stalled() []StalledLine { h.mu.Lock() defer h.mu.Unlock() diff --git a/modules/mesh-delivery-gate/module.json b/modules/mesh-delivery-gate/module.json index 74da6dc0..2129020c 100644 --- a/modules/mesh-delivery-gate/module.json +++ b/modules/mesh-delivery-gate/module.json @@ -33,6 +33,9 @@ "group", "merged" ], + "uses": [ + "operator-channel" + ], "invokes": [ "gitea.gitea_commit_statuses", "gitea.gitea_contains", @@ -57,7 +60,8 @@ { "name": "source", "history": 64 - } + }, + "conditions" ], "reads": [ "mesh-delivery-teller.answers", From 145c54d19c1bcd03cf70784057dd0174cb2507a0 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 25 Oct 2026 18:32:19 +0100 Subject: [PATCH 2/3] Gate settles pull requests closed with no event, looks again at a line's bound, and keeps a waiting head's condition over a restart (issue 1275, review round 1) The conditions the gate now says would otherwise be wrong three ways: - A pull request closed or merged on the forge with no event heard stayed ready here for days (de-spiegel #4, closed 2026-10-17) and would raise a condition. After a complete read of the open list (shorter than the forge tool's 1000), each delivery not final whose pull request is missing is read from the forge and taken as its close or merge; a merge learnt so does not move source.main, as it is undated and could move it back. - A line that comes due on the clock (a state's bound, the author's bound, an answered refusal, an out-of-date refusal turning urgent after 10 minutes) waited for an unrelated event. The gate now asks a look at the earliest such time. - After a restart the heads waiting on the forge start their bound again, so their conditions were cleared and raised again later. A condition is cleared only once the open list was read since start, and not while the forge still shows its head open and waiting. Also: an empty path list ranks with a truncated one in the merge order, and a seat that keeps refusing conditions is retried on the clock 10 looks in a row, then at events only, said once in the log. --- .../cmd/mesh-delivery-gate/conditions.go | 67 ++++++++++- .../cmd/mesh-delivery-gate/holder.go | 10 +- .../cmd/mesh-delivery-gate/issue1275_test.go | 111 ++++++++++++++++++ .../cmd/mesh-delivery-gate/unchecked.go | 75 +++++++++++- 4 files changed, 257 insertions(+), 6 deletions(-) diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/conditions.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/conditions.go index 84bb195e..bbce1e24 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/conditions.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/conditions.go @@ -5,6 +5,7 @@ import ( "fmt" "sort" "strings" + "time" stdio "git.novox.be/novox/mesh-sdk/go" "git.novox.be/novox/mesh-sdk/go/asks" @@ -107,6 +108,16 @@ func (h *Holder) sayConditions() { } h.said = said } + h.mu.Lock() + if at := h.nextStall(h.Now()); !at.IsZero() { + h.lookAt(at, "a delivery whose stalled line, or its severity, comes due with no event") + } + forgeRead := h.forgeRead + waitingOnForge := map[string]bool{} + for id := range h.unchecked { + waitingOnForge[id] = true + } + h.mu.Unlock() lines := map[string]StalledLine{} for _, l := range h.Stalled() { // One condition for an id: the most severe of its lines. @@ -134,10 +145,13 @@ func (h *Holder) sayConditions() { h.Logf("[mesh-delivery] %s is said to the operator as %s: %s", id, severityOfLine(l), l.Says) h.said[key], changed = said, true } + // Cleared only on what was read: not before the forge's open pull requests were read once since the gate started (the + // heads waiting on the forge are kept in memory alone), and not while the forge still shows the head open and waiting: + // its line is gone only because its bound runs again from a later first sight, and it comes back with the same words. var gone []string for key := range h.said { id := strings.TrimSuffix(strings.TrimPrefix(key, "mesh-delivery-gate.delivery."), ".stalled") - if _, ok := lines[id]; !ok { + if _, ok := lines[id]; !ok && forgeRead && !waitingOnForge[id] { gone = append(gone, key) } } @@ -160,7 +174,56 @@ func (h *Holder) sayConditions() { h.Logf("[mesh-delivery] the conditions raised cannot be kept: %v", err) } } - if failed { + if !failed { + h.sayFailures = 0 + return + } + // A seat that keeps refusing is tried again after askAgainAfter, up to sayTries looks in a row; then only at the + // next event, and said once, loudly, in the log (the operator hears nothing while the seat refuses). + h.sayFailures++ + switch { + case h.sayFailures < sayTries: h.again("a condition that could not be said to the operator") + case h.sayFailures == sayTries: + h.Logf("[mesh-delivery] NO CONDITION REACHES THE OPERATOR: the operator-channel seat refused %d looks in a row; "+ + "tried again at each event from now on", sayTries) } } + +// sayTries is how many looks in a row a condition that could not be said is tried again on the clock. +const sayTries = 10 + +// nextStall is the earliest time after now when a delivery's stalled line comes due, or its severity changes, with nothing +// else happening: a state's bound, the author's bound of a rejected head, a head never announced, a refusal the gate +// answers, and an out-of-date refusal turning urgent (1275/AC6). Zero when none. Under the lock. +func (h *Holder) nextStall(now time.Time) time.Time { + var next time.Time + consider := func(at time.Time) { + if at.After(now) && (next.IsZero() || at.Before(next)) { + next = at + } + } + for _, d := range h.deliveries { + if d.State.Final() { + continue + } + if b, bounded := boundOf(d); bounded && b.For > 0 { + consider(boundSince(d).Add(b.For).Add(time.Second)) // pastBound is strictly after the bound + } + r := d.Ready + if r == nil || r.Merged != nil || d.MergedAs != "" { + continue + } + if d.Base == "" && d.Number != 0 { + consider(r.Seen.Add(headHeardBound)) + } + if r.Failed != "" && d.State == Rejected { + consider(d.Since.Add(h.authorBound())) + } + if r.RefusedSince != nil { + consider(r.RefusedSince.Add(h.answeredRefusalBound(d.Repository))) + consider(r.RefusedSince.Add(outOfDateFor)) + } + } + return next +} 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 851305e0..8c4779ca 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go @@ -42,8 +42,12 @@ type Holder struct { // Say says a condition to the operator, and Conditions keeps which the gate raised (conditions.go); nil says none. Say func(c asks.Condition) error Conditions ConditionStore - // said is what Conditions keeps, read at the first look; touched by React alone. - said map[string]string + // said is what Conditions keeps, read at the first look; sayFailures the looks in a row a condition could not be + // said. Touched by React alone. + said map[string]string + sayFailures int + // forgeRead is whether the forge's open pull requests were read since this owner started (SeeOpenPulls). + forgeRead bool // cancels are asks to take back, gathered under the lock and run with the other asks of the look. cancels []func() @@ -958,7 +962,7 @@ func mergeRank(d *Delivery) int { if strings.EqualFold(d.Repository, SDKRepository) { return 0 } - if d.PathsTruncated { + if d.PathsTruncated || len(d.Paths) == 0 { return 2 // its paths are not all known: it may pin, so it goes after those that do not } for _, p := range d.Paths { diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1275_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1275_test.go index 451ea831..91d67e61 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1275_test.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1275_test.go @@ -542,10 +542,121 @@ func TestAnApprovedGroupIsHandedWholeInItsOrderAndAStopIsSaid(t *testing.T) { told := hearConditions(w) told.kept[conditionKeyOf("novox/app@0123456789ab")] = "urgent||" w.settleAll() + if c := told.last("novox/app@0123456789ab"); c != nil { + t.Fatalf("cleared before the forge's open pull requests were read: %+v", c) + } + w.h.wantForge() + w.settleAll() if c := told.last("novox/app@0123456789ab"); c == nil || c.State != asks.StateCleared || len(told.kept) != 0 { t.Fatalf("not cleared: %+v, kept %v", c, told.kept) } }}, + // Review round 1, blocker 1: a pull request closed or merged on the forge with no event heard (de-spiegel #4, + // closed 2026-10-17, ready here eight days later) settles at the next complete read of the open list, and no + // condition is raised for it; a merge learnt so does not move source.main. + {"a pull request gone from the open list settles and raises nothing", func(t *testing.T) { + for _, merged := range []bool{false, true} { + w := newWorld(t) + w.op.auto = false + const repo, head = "novox/de-spiegel", "fdcbaefb3123fdcbaefb3123fdcbaefb3123fdcb" + w.h.PullUpdated(pr(repo, 4, head, "feat/x", "src/a.go")) + w.h.Checked(judgedAt(repo, 4, head, "pass", "t1t1t1t1t1t1")) + w.settleAll() + w.forge.refuse = errors.New("novox/de-spiegel#4 merges into main, which no protection rule requiring the checks guards") + w.op.answer(w.op.open()[0], OptionApprove) + w.settleAll() + id := IDOf(repo, head) + if len(w.h.Stalled()) == 0 { + t.Fatalf("the refused head is not stalled to begin with") + } + p := ForgePull{Number: 4, State: "closed", HeadSHA: head, Base: "main"} + if merged { + p.Merged, p.MergeCommit = true, strings.Repeat("e", 40) + } + w.forge.pulls[repo+"#4"] = p + w.forge.open = []OpenPull{{Owner: "novox", Repo: "other", Number: 1, Base: "main", HeadSHA: "abab"}} + told := hearConditions(w) + moves := len(w.source.moves) + w.h.wantForge() + w.settleAll() + want := Closed + if merged { + want = Merged + } + if st := w.state(id); st != want { + t.Fatalf("merged %v: the delivery is %s, wanted %s", merged, st, want) + } + if c := told.last(id); c != nil { + t.Fatalf("merged %v: a condition was said for it: %+v", merged, c) + } + if len(w.source.moves) != moves { + t.Fatalf("merged %v: source.main moved for a merge learnt by its absence: %+v", merged, w.source.moves) + } + } + }}, + // Review round 1, blocker 2 (1275/AC6): an out-of-date refusal at t0 with no other event is said urgent at t0+10 + // minutes: the gate asks itself for a look then. + {"an out-of-date refusal turns urgent on the clock alone", func(t *testing.T) { + w := approvedGroup(t) + told := hearConditions(w) + allGreen(w) + w.forge.refuse = errAskedAgain + w.op.answer(w.op.open()[0], OptionApprove) + w.settleAll() + t0 := w.h.Now() + w.h.wanted.mu.Lock() + at := w.h.wanted.at + w.h.wanted.mu.Unlock() + if at.IsZero() || at.Sub(t0) > outOfDateFor { + t.Fatalf("no look is wanted by t0+%s: %v", outOfDateFor, at) + } + for i := 0; i < 5 && w.h.Now().Before(t0.Add(outOfDateFor)); i++ { // the timer fires a look, and nothing else happens + w.h.wanted.mu.Lock() + at = w.h.wanted.at + w.h.wanted.mu.Unlock() + if at.IsZero() { + break + } + w.later(at.Sub(w.h.Now())) + w.h.React() + } + c := told.last(IDOf(gCat, gHk)) + if c == nil || c.State != asks.StateRaised || c.Severity != asks.SeverityUrgent { + t.Fatalf("at t0+%s with no event: %+v", w.h.Now().Sub(t0), c) + } + }}, + // Review round 1, blocker 3: a gate that restarts while a head still waits on the forge does not clear its + // condition (the head's first sight is in memory alone, so its bound runs again), and does not say it again. + {"a restart keeps the condition of a head still waiting", func(t *testing.T) { + w := newWorld(t) + told := hearConditions(w) + const head = "bfe82315f42c8f4509d0598c32569a202d5eada6" + id := IDOf("novox/hq", head) + w.forge.open = []OpenPull{{Owner: "novox", Repo: "hq", Number: 243, Base: "main", HeadSHA: head, Required: []string{"mesh/repo-check"}}} + w.h.wantForge() + w.settleAll() + w.later(UncheckedAfter + time.Minute) + w.settleAll() + if c := told.last(id); c == nil || c.State != asks.StateRaised { + t.Fatalf("the unchecked head is not raised: %+v", c) + } + before := len(told.told) + again := newWorldOver(t, w.store, w.forge, w.h.Now()) // the gate restarts + again.h.Say, again.h.Conditions = told.say, told + again.h.wantForge() + again.settleAll() + again.later(time.Minute) + again.settleAll() + if len(told.told) != before { + t.Fatalf("the restart said something of a head still waiting: %+v", told.told[before:]) + } + w.forge.open = nil // it closes + again.h.wantForge() + again.settleAll() + if c := told.last(id); c == nil || c.State != asks.StateCleared { + t.Fatalf("closed, and its condition is not cleared: %+v", c) + } + }}, // The kind behind the day's fault: a decision on a group's state named a word GroupState no longer says. Every word // it says is one groupStatusState knows, and the merge goes on exactly in ready and merging. {"every word of a group's state is decided on", func(t *testing.T) { 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 2c9d92ba..17132c32 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/unchecked.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/unchecked.go @@ -3,6 +3,8 @@ package main import ( "fmt" "slices" + "sort" + "strconv" "strings" "time" ) @@ -175,8 +177,24 @@ func (h *Holder) SeeOpenPulls() error { h.Logf("[mesh-delivery] the forge's open pull requests cannot be read: %v", err) return err } + for _, gone := range h.seeOpen(open) { + h.settleGone(gone) + } + return nil +} + +// openPullsMost is the most open pull requests the forge's gitea_open_pulls answers: it reads 20 pages of 50 and says +// nothing of the rest (mesh-catalog modules/gitea client.ts searchOpenPulls). A list that long may be cut, so what is +// missing from it is not taken as closed. +const openPullsMost = 20 * 50 + +// seeOpen keeps what the forge's open pull requests say, and answers the deliveries whose pull request is not among them: +// open here, closed on the forge with no event heard (a close or merge while the gate was away, or an event lost). Only +// from a complete list. Takes the lock. +func (h *Holder) seeOpen(open []OpenPull) []*Delivery { h.mu.Lock() defer h.mu.Unlock() + h.forgeRead = true now := h.Now() seen := map[string]unchecked{} for _, p := range open { @@ -215,7 +233,62 @@ func (h *Holder) SeeOpenPulls() error { seen[id] = u } h.unchecked = seen - return nil + if len(open) >= openPullsMost { + return nil + } + listed := map[string]bool{} + for _, p := range open { + listed[strings.ToLower(p.Owner+"/"+p.Repo)+"#"+strconv.Itoa(p.Number)] = true + } + var gone []*Delivery + for _, d := range h.deliveries { + if d.State.Final() || d.MergedAs != "" || d.Number <= 0 || listed[strings.ToLower(d.Repository)+"#"+strconv.Itoa(d.Number)] { + continue + } + gone = append(gone, d) + } + sort.Slice(gone, func(i, j int) bool { return gone[i].ID < gone[j].ID }) + return gone +} + +// settleGone reads from the forge a pull request the open list does not hold, and takes its merge or its close as the +// forge's event would have said it (issue 1275, review round 1: a pull request closed on 2026-10-17 was still ready here +// eight days later). A merge learnt this way is past and undated: its delivery is merged, and source.main is not moved +// for it (it could move it back); the trunk's newer merges and the catch-up at start move source.main. A forge that +// cannot say changes nothing. Outside the lock. +func (h *Holder) settleGone(d *Delivery) { + h.mu.Lock() + id, owner, repo, number, head, base, branch := d.ID, d.Owner(), d.Repo(), d.Number, d.Commit, d.Base, d.Branch + h.mu.Unlock() + fc, err := h.Forge.Statuses(owner, repo, number, head, base) + if err != nil || fc == nil || fc.Pull == nil { + if err != nil { + h.Logf("[mesh-delivery] %s is not among the forge's open pull requests, and the forge cannot say what became of it: %v", id, err) + } + return + } + p := fc.Pull + if p.State != "closed" || p.Number != number { + return // open again, a state the forge did not say, or not this pull request: the next reading says + } + ev := PullEvent{Owner: owner, Repo: repo, Number: number, Base: base, Head: branch, HeadSHA: head, HTMLURL: p.HTMLURL} + if p.Merged && p.MergeCommit != "" { + ev.MergeCommit, ev.Merged = p.MergeCommit, true + h.Logf("[mesh-delivery] %s is not among the forge's open pull requests: the forge says it merged as %s", id, shortOf(p.MergeCommit, 12)) + h.PullMerged(ev) + h.mu.Lock() + if m := h.deliveries[id]; m != nil && m.MergedAs != "" { + m.InSource = true + if m.Ready != nil { + m.Ready.SourceMoved = true + } + h.keep(m) + } + h.mu.Unlock() + return + } + h.Logf("[mesh-delivery] %s is not among the forge's open pull requests: the forge says it was closed unmerged", id) + h.PullClosed(ev) } // started is whether the head's check started, rather than was never asked: it has any merge check, or a delivery From 0c216d99d0c0e920d5e680300b37b707a31ee274 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 25 Oct 2026 18:37:59 +0100 Subject: [PATCH 3/3] Gate takes a merge learnt after the fact as past: no trunk move, the forge's time, never source.main (issue 1275, review round 2) A pull request found merged only by its absence from the open list went through PullMerged, which moved every other open delivery of its repository back to an old trunk (ready ones to proposed, composed checks out of date), dated the merge now, and so let a later catch-up write the old commit into source.main. pullMerged now takes such a merge as late: its own delivery is merged and its group closed, no other trunk moves, its time is the forge's or none, and LearntLate keeps it out of source.main in finish, settleForgeMerges and the catch-up. The gitea tool gitea_commit_statuses now answers the pull request's merged_at, and the gate reads a gone pull request by its number alone, so a deleted branch does not fail the read. --- modules/gitea/tools/index.ts | 4 +- .../cmd/mesh-delivery-gate/holder.go | 19 ++++++-- .../cmd/mesh-delivery-gate/issue1275_test.go | 45 +++++++++++++++++++ .../cmd/mesh-delivery-gate/merges.go | 10 ++++- .../cmd/mesh-delivery-gate/model.go | 7 ++- .../cmd/mesh-delivery-gate/ports.go | 4 +- .../cmd/mesh-delivery-gate/unchecked.go | 25 ++++------- 7 files changed, 88 insertions(+), 26 deletions(-) diff --git a/modules/gitea/tools/index.ts b/modules/gitea/tools/index.ts index e22a4965..3f36f29a 100644 --- a/modules/gitea/tools/index.ts +++ b/modules/gitea/tools/index.ts @@ -508,11 +508,11 @@ function allGiteaTools(gitea: GiteaClient, admin?: () => GiteaAdmin): ToolDefini if (!owner || !repo) throw new Error("name the repository: owner and repo"); if (!Number.isInteger(number) || number < 0) throw new Error(`${args.number} is not a pull request's number`); let pull: { number: number; title: string; state: string; merged: boolean; base?: string; head?: string; - head_sha?: string; merge_commit_sha?: string; html_url: string } | undefined; + head_sha?: string; merge_commit_sha?: string; merged_at?: string; html_url: string } | undefined; if (number > 0) { const p = await gitea.getPullRequest(owner, repo, number); pull = { number: p.number, title: p.title, state: p.state, merged: p.merged, base: p.base, head: p.head, - head_sha: p.head_sha, merge_commit_sha: p.merge_commit_sha, html_url: p.html_url }; + head_sha: p.head_sha, merge_commit_sha: p.merge_commit_sha, merged_at: p.merged_at, html_url: p.html_url }; if (!ref) ref = p.head_sha ?? ""; if (!base) base = p.base ?? ""; if (!ref) throw new Error(`${owner}/${repo}#${number} names no head commit`); 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 8c4779ca..505f36a5 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go @@ -646,7 +646,12 @@ func (h *Holder) leaveGroup(d *Delivery) { // PullMerged takes a merge: the commit it landed as, and the delivery it was — made from the forge's word // when this owner never heard its head (merged before it held the seat). -func (h *Holder) PullMerged(p PullEvent) { +func (h *Holder) PullMerged(p PullEvent) { h.pullMerged(p, false) } + +// pullMerged takes a merge. late is a merge learnt after the fact (settleGone): its own delivery is merged and its group +// closed, but no other delivery's trunk moves (the trunk has long moved past it, by merges heard as they happened), +// its time is the forge's or none (never now), and it never moves source.main (LearntLate). +func (h *Holder) pullMerged(p PullEvent, late bool) { if p.MergeCommit == "" { h.Refuse(errors.New("a merge announced with no merge commit; nothing published")) return @@ -721,16 +726,24 @@ func (h *Holder) PullMerged(p PullEvent) { d.MergedAs = p.MergeCommit if t, err := time.Parse(time.RFC3339, p.MergedAt); err == nil { d.MergedAt = t.UTC() - } else { + } else if !late { d.MergedAt = now } + if late { + d.LearntLate, d.InSource = true, true + if d.Ready != nil { + d.Ready.SourceMoved = true + } + } ts = append(ts, settle(d, Facts{Now: now})...) if d.State == Held && d.HeldWhy == "" { d.HeldWhy = "merged without a passing check: only a person decides what follows" } h.moved(d, ts) h.closeGroupOf(d) - h.trunkMoved(d, p.Base, p.MergeCommit, now) + if !late { + h.trunkMoved(d, p.Base, p.MergeCommit, now) + } } // trunkMoved takes a merge into a trunk as that trunk's move (novox/hq ADR 0323 §7): every other open delivery into diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1275_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1275_test.go index 91d67e61..83ecc256 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1275_test.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1275_test.go @@ -2,6 +2,7 @@ package main import ( "errors" + "fmt" "strings" "sync" "testing" @@ -594,6 +595,50 @@ func TestAnApprovedGroupIsHandedWholeInItsOrderAndAStopIsSaid(t *testing.T) { } } }}, + // Review round 2: a merge learnt after the fact moves no other delivery's trunk (an approved group of the same + // repository keeps its trunk, its state and its composed check), takes the forge's time, and a catch-up after it + // does not move source.main. + {"a merge learnt late moves no trunk and no source", func(t *testing.T) { + w := approvedGroup(t) + allGreen(w) + w.merges.manual = true + w.op.answer(w.op.open()[0], OptionApprove) + w.settleAll() + const lateHead, lateMerge = "9a9a9a9a9a9a9a9a9a9a9a9a9a9a9a9a9a9a9a9a", "8b8b8b8b8b8b8b8b8b8b8b8b8b8b8b8b8b8b8b8b" + w.h.PullUpdated(pr(gCat, 9, lateHead, "fix/old", "modules/app/x.go")) + w.h.Checked(judged(gCat, 9, lateHead, "pass", gTrunk0)) + w.settleAll() + late := IDOf(gCat, lateHead) + trunkOf := func() map[string]string { + out := map[string]string{} + for _, id := range []string{IDOf(gCat, gHk), IDOf(gCtl, gHc)} { + out[id] = w.delivery(id).TrunkNow + " " + string(w.delivery(id).State) + } + return out + } + before, moves := trunkOf(), len(w.source.moves) + w.forge.pulls[gCat+"#9"] = ForgePull{Number: 9, State: "closed", Merged: true, MergeCommit: lateMerge, + MergedAt: "2026-10-01T09:00:00Z", HeadSHA: lateHead, Base: "main"} + w.forge.open = []OpenPull{{Owner: "novox", Repo: "mesh-catalog", Number: 2, Base: "main", HeadSHA: gHk}, + {Owner: "novox", Repo: "mesh-controller", Number: 1, Base: "main", HeadSHA: gHc}} + w.h.wantForge() + w.settleAll() + d := w.delivery(late) + if d.State != Merged || !d.MergedAt.Equal(time.Date(2026, 10, 1, 9, 0, 0, 0, time.UTC)) || !d.LearntLate { + t.Fatalf("the late merge is %s at %v (learnt late %v)", d.State, d.MergedAt, d.LearntLate) + } + if after := trunkOf(); fmt.Sprint(after) != fmt.Sprint(before) { + t.Fatalf("the group's trunk or state moved: %v, was %v", after, before) + } + if g := w.h.groups["feat/g"]; g.Check == nil || g.Check.OutOfDate != "" { + t.Fatalf("the group's composed check went out of date: %+v", g.Check) + } + w.h.SourceCatchUp() + w.settleAll() + if len(w.source.moves) != moves { + t.Fatalf("source.main moved for a merge learnt late: %+v", w.source.moves[moves:]) + } + }}, // Review round 1, blocker 2 (1275/AC6): an out-of-date refusal at t0 with no other event is said urgent at t0+10 // minutes: the gate asks itself for a look then. {"an out-of-date refusal turns urgent on the clock alone", func(t *testing.T) { diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/merges.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/merges.go index 3d7a4499..e7e5f4f9 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/merges.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/merges.go @@ -356,6 +356,9 @@ func (h *Holder) settleMerges() []func() { var members []string for hid := range r.Heads { m := h.deliveries[hid] + if m.LearntLate { + continue // learnt after the fact: never written to source.main (settleGone) + } commit := m.MergedAs if m.Ready != nil && m.Ready.MergeCommit != "" { commit = m.Ready.MergeCommit @@ -363,6 +366,9 @@ func (h *Holder) settleMerges() []func() { heads[m.Repository] = commit members = append(members, hid) } + if len(members) == 0 { + continue // every member learnt after the fact: nothing to write + } sort.Strings(members) last := h.deliveries[members[len(members)-1]] out = append(out, func() { h.finish(name, key, approved, last.ID, heads, members) }) @@ -441,7 +447,7 @@ func (h *Holder) settleForgeMerges() []func() { again = h.newestInSource() } for _, d := range h.deliveries { - if d.MergedAs == "" || (d.InSource && !again[d]) || gateMerges(d) { + if d.MergedAs == "" || (d.InSource && !again[d]) || gateMerges(d) || d.LearntLate { continue } if d.Base != MergeBranch || d.replayed() || h.newerMergeOf(d) { @@ -558,7 +564,7 @@ func (h *Holder) SourceCatchUp() { func (h *Holder) newestInSource() map[*Delivery]bool { newest := map[string]*Delivery{} for _, d := range h.deliveries { - if d.MergedAs == "" || !d.InSource || gateMerges(d) || d.Base != MergeBranch || d.replayed() { + if d.MergedAs == "" || !d.InSource || gateMerges(d) || d.LearntLate || d.Base != MergeBranch || d.replayed() { continue } if o := newest[d.Repository]; o == nil || mergedWhen(d).After(mergedWhen(o)) { 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 fdbc961c..9f2a4de8 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/model.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/model.go @@ -82,8 +82,11 @@ type Delivery struct { MergedAt time.Time `json:"merged_at"` // InSource says the merge was looked at for source.main: written there, or left as older than the trunk's head (issue 888). // A merge the gate handed to the bridge on an Approve is moved there by finish instead (merges.go). - InSource bool `json:"in_source,omitempty"` - HeldWhy string `json:"held_why,omitempty"` + InSource bool `json:"in_source,omitempty"` + // LearntLate says its merge was learnt after the fact, from its absence among the forge's open pull requests + // (settleGone, issue 1275): it moved no trunk here, and it never moves source.main, not even at a catch-up. + LearntLate bool `json:"learnt_late,omitempty"` + HeldWhy string `json:"held_why,omitempty"` // FromForge is a delivery made from the forge's word at its merge: its head was never heard by this owner. FromForge bool `json:"from_forge,omitempty"` diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ports.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ports.go index a2e8fb92..83d95a56 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ports.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ports.go @@ -45,7 +45,9 @@ type ForgePull struct { Head string `json:"head,omitempty"` HeadSHA string `json:"head_sha,omitempty"` MergeCommit string `json:"merge_commit_sha,omitempty"` - HTMLURL string `json:"html_url,omitempty"` + // MergedAt is when it merged, as the forge says it (RFC 3339); empty when not merged or not said. + MergedAt string `json:"merged_at,omitempty"` + HTMLURL string `json:"html_url,omitempty"` } // ForgeStatus is one status of a commit, the newest of its context. 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 17132c32..368b3271 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/unchecked.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/unchecked.go @@ -253,14 +253,15 @@ func (h *Holder) seeOpen(open []OpenPull) []*Delivery { // settleGone reads from the forge a pull request the open list does not hold, and takes its merge or its close as the // forge's event would have said it (issue 1275, review round 1: a pull request closed on 2026-10-17 was still ready here -// eight days later). A merge learnt this way is past and undated: its delivery is merged, and source.main is not moved -// for it (it could move it back); the trunk's newer merges and the catch-up at start move source.main. A forge that -// cannot say changes nothing. Outside the lock. +// eight days later). A merge learnt this way is past: its delivery is merged with the forge's time, moves no other +// delivery's trunk, and never moves source.main (pullMerged, late); the trunk's merges heard as they happened move +// those. A forge that cannot say changes nothing. Outside the lock. func (h *Holder) settleGone(d *Delivery) { h.mu.Lock() id, owner, repo, number, head, base, branch := d.ID, d.Owner(), d.Repo(), d.Number, d.Commit, d.Base, d.Branch h.mu.Unlock() - fc, err := h.Forge.Statuses(owner, repo, number, head, base) + // Read by its number alone: the forge reads the pull request's own head, which it keeps after the branch is gone. + fc, err := h.Forge.Statuses(owner, repo, number, "", base) if err != nil || fc == nil || fc.Pull == nil { if err != nil { h.Logf("[mesh-delivery] %s is not among the forge's open pull requests, and the forge cannot say what became of it: %v", id, err) @@ -273,18 +274,10 @@ func (h *Holder) settleGone(d *Delivery) { } ev := PullEvent{Owner: owner, Repo: repo, Number: number, Base: base, Head: branch, HeadSHA: head, HTMLURL: p.HTMLURL} if p.Merged && p.MergeCommit != "" { - ev.MergeCommit, ev.Merged = p.MergeCommit, true - h.Logf("[mesh-delivery] %s is not among the forge's open pull requests: the forge says it merged as %s", id, shortOf(p.MergeCommit, 12)) - h.PullMerged(ev) - h.mu.Lock() - if m := h.deliveries[id]; m != nil && m.MergedAs != "" { - m.InSource = true - if m.Ready != nil { - m.Ready.SourceMoved = true - } - h.keep(m) - } - h.mu.Unlock() + ev.MergeCommit, ev.Merged, ev.MergedAt = p.MergeCommit, true, p.MergedAt + h.Logf("[mesh-delivery] %s is not among the forge's open pull requests: the forge says it merged as %s at %s", id, + shortOf(p.MergeCommit, 12), orUnknown(p.MergedAt)) + h.pullMerged(ev, true) return } h.Logf("[mesh-delivery] %s is not among the forge's open pull requests: the forge says it was closed unmerged", id)