From f61e00aeaa331e0c411f50c33331dc2662a1c534 Mon Sep 17 00:00:00 2001 From: "fix-deliv-awg@novox" Date: Fri, 16 Oct 2026 21:29:55 +0200 Subject: [PATCH] =?UTF-8?q?Ask=20the=20operator=20to=20Approve=20a=20merge?= =?UTF-8?q?=20into=20main=20and=20merge=20only=20on=20the=20warrant=20(iss?= =?UTF-8?q?ue=20948,=20ADR=200398=20=C2=A73)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../cmd/mesh-delivery/approval.go | 408 ++++++++++++++++++ .../cmd/mesh-delivery/approval_test.go | 202 +++++++++ .../cmd/mesh-delivery/fakes_test.go | 5 + .../mesh-delivery/cmd/mesh-delivery/holder.go | 7 +- .../mesh-delivery/cmd/mesh-delivery/main.go | 2 + .../cmd/mesh-delivery/manifest_test.go | 8 +- .../mesh-delivery/cmd/mesh-delivery/model.go | 2 + .../cmd/mesh-delivery/operator_test.go | 104 +++++ .../mesh-delivery/cmd/mesh-delivery/ready.go | 62 ++- modules/mesh-delivery/go.mod | 2 +- modules/mesh-delivery/go.sum | 4 +- modules/mesh-delivery/module.json | 9 +- 12 files changed, 806 insertions(+), 9 deletions(-) create mode 100644 modules/mesh-delivery/cmd/mesh-delivery/approval.go create mode 100644 modules/mesh-delivery/cmd/mesh-delivery/approval_test.go create mode 100644 modules/mesh-delivery/cmd/mesh-delivery/operator_test.go diff --git a/modules/mesh-delivery/cmd/mesh-delivery/approval.go b/modules/mesh-delivery/cmd/mesh-delivery/approval.go new file mode 100644 index 00000000..7989ddd3 --- /dev/null +++ b/modules/mesh-delivery/cmd/mesh-delivery/approval.go @@ -0,0 +1,408 @@ +package main + +import ( + "fmt" + "regexp" + "sort" + "strconv" + "strings" + "time" + + "git.novox.be/novox/mesh-sdk/go/asks" +) + +// A merge into main waits for the operator's Approve (novox/hq ADR 0398 §3, issue 948). +// +// Where a ready mark at the head, a passing check and the group's turn used to call the merge tool at once, this +// owner now asks the operator on the router (the operator channel, as the knowledge base asks for a record): Approve +// or Decline, each option binding the act "merge these heads". It merges only after an Approve whose warrant holds for +// that ask — the same asker, the same ask's digest, an option it offered, in time, and the act that option bound. +// +// - Asked once per head. A group is asked once, for all its members' heads together, by the first member that reaches +// this point; the Approve covers every member at those heads. +// - A push after the ask cancels it (the mark is cleared with it): the new head needs a new mark and a new Approve. +// - A Decline merges nothing, is said once on the pull request, and clears the mark. +// - An ask that ended without a choice (expired, cancelled, refused by the router) is asked again at the next tick. +// - The ask, its state and the warrant's words are kept on the delivery's ReadyRun and shown by `show`. +// +// The terminal Approve (ADR 0398 §3) is issue 953; today the proving channel is the operator's phone. + +// Module is this module's name: the one the bus grants its asks under. +const Module = "mesh-delivery" + +// The two answers, both at the level approve. +const ( + OptionApprove = "approve" + OptionDecline = "decline" +) + +// AskLasts is how long the operator has to answer: a day less a margin, under the router's bound for an approval. +const AskLasts = asks.ApproveLasts - 10*time.Minute + +// Operator is the operator channel as this owner reaches it: asker.Client in the running module. +type Operator interface { + // Ask checks, keeps and publishes an ask under this module's name. + Ask(a asks.Ask) error + // Cancel takes an ask back. + Cancel(id string) error + // Where is an ask's state as the router keeps it, and its warrant once there is one. + Where(id string) (string, *asks.Warrant, error) +} + +// headsString is the heads an act names, as an act's value: "=" for each, sorted, joined by commas. +func headsString(heads map[string]string) string { + var out []string + for id, sha := range heads { + out = append(out, id+"="+sha) + } + sort.Strings(out) + return strings.Join(out, ",") +} + +// mergeAct is what an answer binds: this ask, the answer, and the heads it covers. +func mergeAct(ask, option string, heads map[string]string) asks.Act { + return asks.Act{"verb": Module + ".merge", "ask": ask, "option": option, "heads": headsString(heads)} +} + +var unshownPath = regexp.MustCompile(`[/\\@~$]|\d+\.\d+\.\d+|[A-Za-z0-9+/=_]{24,}|(?i)\b[a-z0-9-]+\.[a-z]{2,12}\b`) + +// phone is a line the operator's phone may be shown: what the router refuses in an ask's words (a path, an address, a +// host-like name, a long unbroken word) is left out, and the line is cut to n characters. +func phone(s string, n int) string { + s = strings.Join(strings.Fields(unshownPath.ReplaceAllString(s, "…")), " ") + if r := []rune(s); len(r) > n { + return string(r[:n-1]) + "…" + } + return s +} + +var issueNamed = regexp.MustCompile(`(?i)\bissues?\s*#?(\d+)`) + +// IssuesIn reads the issues a pull request's title and body name ("issue 948"), ascending, once each. +func IssuesIn(text string) []int { + seen := map[int]bool{} + var out []int + for _, m := range issueNamed.FindAllStringSubmatch(text, -1) { + if n, err := strconv.Atoi(m[1]); err == nil && !seen[n] { + seen[n] = true + out = append(out, n) + } + } + sort.Ints(out) + return out +} + +func issuesSaid(ds []*Delivery) string { + seen := map[int]bool{} + var out []string + for _, d := range ds { + for _, n := range d.Issues { + if !seen[n] { + seen[n] = true + out = append(out, "issue "+strconv.Itoa(n)) + } + } + } + if len(out) == 0 { + return "none named" + } + return strings.Join(out, ", ") +} + +func repoName(repository string) string { + _, name, ok := strings.Cut(repository, "/") + if !ok { + return repository + } + return name +} + +func pullSaid(d *Delivery) string { return fmt.Sprintf("%s#%d", repoName(d.Repository), d.Number) } + +// composeMergeAsk is the ask for a delivery, or for a group's members together (in the order they merge). +func composeMergeAsk(id string, d *Delivery, group string, members []*Delivery, heads map[string]string, now time.Time) asks.Ask { + var a asks.Ask + a.ID, a.Who, a.Expires = id, asks.Operator, now.Add(AskLasts) + a.OnExpiry = "nothing is merged; the ask is made again" + a.About = "delivery." + phone(repoName(d.Repository), 40) + "." + strconv.Itoa(d.Number) + a.About = strings.ReplaceAll(a.About, " ", "-") + var reviewers, reaches, details []string + for _, m := range members { + by, at := "a reviewer", "" + if m.Ready != nil { + if m.Ready.By != "" { + by = m.Ready.By + } + at = m.Ready.MarkedAt + } + reviewers = append(reviewers, fmt.Sprintf("%s marked %s ready at %s", by, pullSaid(m), shortOf(m.Commit, 12))) + plan := "no delivery plan yet" + if m.Plan != nil && m.Plan.Summary != "" { + plan = m.Plan.Summary + } + reaches = append(reaches, plan) + details = append(details, fmt.Sprintf("%s %q at %s: issues: %s; reviewer: %s, marked %s; reaches: %s", + pullSaid(m), m.Title, shortOf(m.Commit, 12), issuesSaid([]*Delivery{m}), by, at, plan)) + } + if group == "" { + a.Headline = phone("Merge into main: "+pullSaid(d)+" "+d.Title, asks.HeadlineLength) + a.Explanation = strings.Join([]string{ + phone(fmt.Sprintf("%s %s passed its checks and was reviewed. Approve merges it into main at %s; Decline leaves it unmerged.", + pullSaid(d), d.Title, shortOf(d.Commit, 12)), asks.SummaryLength), + phone("Issues: "+issuesSaid(members), asks.PointLength), + phone("Reviewer: "+reviewers[0], asks.PointLength), + phone("Reaches: "+reaches[0], asks.PointLength), + }, "\n") + } else { + var names []string + for _, m := range members { + names = append(names, pullSaid(m)) + } + a.Headline = phone("Merge into main: group "+group+" ("+strings.Join(names, ", ")+")", asks.HeadlineLength) + a.Explanation = strings.Join([]string{ + phone(fmt.Sprintf("The group %s, %d pull requests, passed its checks and was reviewed. Approve merges all of them into main in their order, at the heads named; Decline leaves them unmerged.", + group, len(members)), asks.SummaryLength), + phone("Members, in order: "+strings.Join(names, ", "), asks.PointLength), + phone("Issues: "+issuesSaid(members), asks.PointLength), + phone("Reaches: "+strings.Join(reaches, "; "), asks.PointLength), + }, "\n") + } + a.Details = phone(strings.Join(details, "\n"), 1500) + if len(details) > 0 { // keep the lines apart: phone joined them + a.Details = strings.Join(func() []string { + var out []string + for _, l := range details { + out = append(out, phone(l, 400)) + } + return out + }(), "\n") + } + for _, o := range []struct{ id, label, does string }{ + {OptionApprove, "Approve", "the pull request is merged into main at the head named (a group: all its members, in order)"}, + {OptionDecline, "Decline", "nothing is merged and the ready mark is taken off"}, + } { + binds, _ := asks.ActDigest(mergeAct(id, o.id, heads)) + a.Options = append(a.Options, asks.Option{ID: o.id, Label: o.label, Does: o.does, Level: asks.Approve, Binds: binds}) + } + return a +} + +// askMembers are the deliveries one ask covers: the group's members not yet merged, in the group's order, or the delivery. +func (h *Holder) askMembers(d *Delivery) []*Delivery { + g := h.groups[d.Group] + if d.Group == "" || g == nil { + return []*Delivery{d} + } + var out []*Delivery + seen := map[string]bool{} + for _, id := range append(append([]string(nil), g.Order...), g.Members...) { + if o := h.deliveries[id]; o != nil && !seen[id] && o.MergedAs == "" && !o.State.Final() { + seen[id] = true + out = append(out, o) + } + } + if len(out) == 0 { + return []*Delivery{d} + } + return out +} + +// covering is the ready run holding an ask (pending or approved) that covers d at its head: its own, or a group +// sibling's, whose ask named d's head too. +func (h *Holder) covering(d *Delivery) *ReadyRun { + has := func(o *Delivery) *ReadyRun { + if o != nil && o.Ready != nil && (o.Ready.AskID != "" || o.Ready.Approved != nil) && o.Ready.Heads[d.ID] == d.Commit { + return o.Ready + } + return nil + } + if r := has(d); r != nil { + return r + } + if g := h.groups[d.Group]; d.Group != "" && g != nil { + for _, id := range g.Members { + if r := has(h.deliveries[id]); r != nil { + return r + } + } + } + return nil +} + +// approval is what a delivery that may merge waits on for the operator: empty once its head is approved, and the ask +// to make when none is made yet. Under the lock. +func (h *Holder) approval(d *Delivery, now time.Time) (string, func()) { + r := d.Ready + if c := h.covering(d); c != nil { + if c.Approved != nil { + r.Approved, r.Warrant, r.AskID, r.Ask, r.Heads = c.Approved, c.Warrant, c.AskID, c.Ask, c.Heads + return "", nil + } + return "the operator's Approve is asked (" + c.AskID + ")", nil + } + if r.Refused != "" { + return "the operator's answer was refused here: " + r.Refused, nil + } + if h.Operator == nil { + return "the operator cannot be asked from here", nil + } + members := h.askMembers(d) + group := "" + if len(members) > 1 { + group = d.Group + } + heads := map[string]string{} + for _, m := range members { + heads[m.ID] = m.Commit + } + r.Asks++ + id := fmt.Sprintf("merge-%d-%s-%d", d.Number, shortOf(d.Commit, 12), r.Asks) + ask := composeMergeAsk(id, d, group, members, heads, now) + taken := []*Delivery{} + for _, m := range members { + if m.Ready == nil { + continue + } + m.Ready.AskID, m.Ready.Ask, m.Ready.Heads, m.Ready.AskedAt, m.Ready.Asks = id, &ask, heads, now, max(m.Ready.Asks, r.Asks) + h.keep(m) + taken = append(taken, m) + } + h.Logf("[mesh-delivery] %s is marked ready and its checks passed: the operator is asked to Approve its merge into main (%s, ask %d)", + d.ID, id, r.Asks) + return "the operator's Approve is asked (" + id + ")", func() { + if err := h.Operator.Ask(ask); err != nil { + h.mu.Lock() + defer h.mu.Unlock() + for _, m := range taken { + if m.Ready != nil && m.Ready.AskID == id { + m.Ready.AskID, m.Ready.Ask, m.Ready.Heads = "", nil, nil + h.keep(m) + } + } + h.Logf("[mesh-delivery] the operator could not be asked about %s (%v); asked again at the next tick", d.ID, err) + } + } +} + +// dropAsk forgets a pending ask on every delivery it covers and takes it back from the router; under the lock. +func (h *Holder) dropAsk(id, why string) func() { + for _, d := range h.deliveries { + if d.Ready != nil && d.Ready.AskID == id { + d.Ready.AskID, d.Ready.Ask, d.Ready.Heads = "", nil, nil + h.keep(d) + } + } + h.Logf("[mesh-delivery] the operator's ask %s is cancelled: %s", id, why) + return func() { + if h.Operator == nil { + return + } + if err := h.Operator.Cancel(id); err != nil { + h.Logf("[mesh-delivery] the operator's ask %s could not be cancelled (%v)", id, err) + } + } +} + +// pendingAsks are the asks waiting for an answer, by id; under the lock. +func (h *Holder) pendingAsks() []string { + seen := map[string]bool{} + var out []string + for _, d := range h.deliveries { + if r := d.Ready; r != nil && r.AskID != "" && r.Approved == nil && r.Declined == nil && !seen[r.AskID] { + seen[r.AskID] = true + out = append(out, r.AskID) + } + } + sort.Strings(out) + return out +} + +// hearAsks reads the router's record of every pending ask and applies the warrant of each that closed. +func (h *Holder) hearAsks() { + if h.Operator == nil { + return + } + h.mu.Lock() + ids := h.pendingAsks() + h.mu.Unlock() + for _, id := range ids { + state, warrant, err := h.Operator.Where(id) + if err != nil { + h.Logf("[mesh-delivery] the router's record of the ask %s cannot be read: %v", id, err) + continue + } + if state == "" || state == "open" || state == "held" { + continue + } + h.mu.Lock() + said := h.decided(id, state, warrant) + h.mu.Unlock() + for _, s := range said { + s() + } + } +} + +// decided applies one closed ask's warrant to the deliveries it covers; under the lock. It returns what to say on the +// pull requests. +func (h *Holder) decided(id, state string, w *asks.Warrant) []func() { + var covered []*Delivery + for _, d := range h.deliveries { + if d.Ready != nil && d.Ready.AskID == id { + covered = append(covered, d) + } + } + if len(covered) == 0 { + return nil + } + sort.Slice(covered, func(i, j int) bool { return covered[i].ID < covered[j].ID }) + first, now := covered[0].Ready, h.Now() + end := func(why string) { + for _, d := range covered { + d.Ready.AskID, d.Ready.Ask, d.Ready.Heads = "", nil, nil + h.keep(d) + } + h.Logf("[mesh-delivery] the operator's ask %s ended without a choice (%s); asked again at the next tick", id, why) + } + if w == nil || w.Outcome != asks.OutcomeChosen { + why := "the router says " + state + if w != nil { + why = "ended " + string(w.Outcome) + if w.Words != "" { + why += " (" + w.Words + ")" + } + } + end(why) + return nil + } + option, err := w.For(Module, *first.Ask) + if err == nil { + err = option.Performs(mergeAct(id, option.ID, first.Heads)) + } + if err != nil { + h.Logf("[mesh-delivery] REFUSED the answer to the ask %s: %v", id, err) + for _, d := range covered { + d.Ready.AskID, d.Ready.Ask, d.Ready.Heads = "", nil, nil + d.Ready.Refused = "was answered, and the answer does not hold here: " + clip(oneLine(err.Error()), 300) + h.keep(d) + } + return nil + } + var said []func() + for _, d := range covered { + r := d.Ready + r.Warrant = w.Says() + switch option.ID { + case OptionApprove: + r.Approved = &now + h.Logf("[mesh-delivery] %s: the operator approved its merge into main (%s)", d.ID, r.Warrant) + case OptionDecline: + r.Declined = &now + h.Logf("[mesh-delivery] %s: the operator declined its merge into main (%s)", d.ID, r.Warrant) + said = append(said, h.sayOn(d, fmt.Sprintf("mesh-delivery: **not merged.** The merge of this pull request into main at %s was "+ + "declined: %s. The ready mark is taken off; a new mark, once reviewed, asks again.", d.Commit, r.Warrant))) + } + h.keep(d) + } + return said +} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/approval_test.go b/modules/mesh-delivery/cmd/mesh-delivery/approval_test.go new file mode 100644 index 00000000..33926550 --- /dev/null +++ b/modules/mesh-delivery/cmd/mesh-delivery/approval_test.go @@ -0,0 +1,202 @@ +package main + +import ( + "strings" + "testing" + "time" + + "git.novox.be/novox/mesh-sdk/go/asks" +) + +// **A merge into main waits for the operator's Approve** (novox/hq ADR 0398 §3, issue 948): a marked, green delivery +// into main is asked of the operator, and merged only after an Approve whose warrant holds for that ask. + +// markedGreen is a pull request into main, announced, judged green and marked ready at its head; the operator is not +// answering by himself (w.op.auto is off). +func markedGreen(t *testing.T) *world { + w := newWorld(t) + w.op.auto = false + p := pr("novox/app", 7, readyHead, "feat/x", "src/a.go") + p.Title = "Fix the apostrophe (issue 948, issue 12)" + w.h.PullUpdated(p) + w.h.Checked(judgedAt("novox/app", 7, readyHead, "pass", "t1t1t1t1t1t1")) + w.forge.open = []OpenPull{markedPull("novox/app", 7, readyHead, readyHead)} + w.h.SeeOpenPulls() + w.settleAll() + return w +} + +func TestAMarkedGreenDeliveryIntoMainIsAskedAndNotMergedUntilApproved(t *testing.T) { + w := markedGreen(t) + id := IDOf("novox/app", readyHead) + if len(w.forge.merges) != 0 { + t.Fatalf("merged before the operator's Approve: %v", w.forge.merges) + } + open := w.op.open() + if len(open) != 1 { + t.Fatalf("the operator is asked %v; wanted exactly one ask", open) + } + a := w.op.ask(open[0]) + if !strings.HasPrefix(a.Headline, "Merge into main: app#7") || a.Who != asks.Operator || + len(a.Options) != 2 || a.Options[0].ID != OptionApprove || a.Options[1].ID != OptionDecline { + t.Fatalf("the ask is not the merge ask: %+v", a) + } + for _, want := range []string{"issue 948", "issue 12", "reviewer", "builds app"} { + if !strings.Contains(a.Explanation, want) { + t.Fatalf("the ask does not say %q: %s", want, a.Explanation) + } + } + if err := asks.Glance(a.Explanation); err != nil { + t.Fatalf("the ask does not read at a glance: %v", err) + } + // Ticks and minutes pass without an answer: not merged, not asked again. + for range 3 { + w.later(2 * time.Minute) + w.settleAll() + } + if len(w.forge.merges) != 0 || w.op.count() != 1 { + t.Fatalf("while waiting: merges %v, asks %d", w.forge.merges, w.op.count()) + } + if d := w.delivery(id); d.Ready.AskID != open[0] || !strings.Contains(d.Ready.Waits, "Approve") { + t.Fatalf("the ask is not kept on the delivery: %+v", d.Ready) + } + w.op.answer(open[0], OptionApprove) + w.settleAll() + if len(w.forge.merges) != 1 || w.forge.merges[0] != "novox/app#7@"+readyHead { + t.Fatalf("after the Approve it was merged %v; wanted once, at the marked head", w.forge.merges) + } + d := w.delivery(id) + if d.Ready.Approved == nil || !strings.Contains(d.Ready.Warrant, "chose Approve") { + t.Fatalf("the warrant is not kept: %+v", d.Ready) + } + w.h.PullMerged(merged("novox/app", 7, readyHead, "m7m7m7m7m7m7")) + w.h.WalkMoved(w.ctl.put(aWalk("plan-7", "novox/app", "m7m7m7m7m7m7", true))) + w.settleAll() + var last Transition + for _, tr := range w.delivery(id).Transitions { + if tr.Event == EvMerged { + last = tr + } + } + if !strings.Contains(last.By, "approved: the operator, via telegram") { + t.Fatalf("the merge's transition does not name the operator's Approve: %+v", last) + } +} + +func TestAnAnswerThatIsNotForThisAskMergesNothing(t *testing.T) { + w := markedGreen(t) + id := w.op.open()[0] + w.op.answer(id, OptionApprove) + w.op.mu.Lock() + w.op.warrant[id].AskDigest = "sha256:not-the-ask-that-was-shown" + w.op.mu.Unlock() + w.settleAll() + if len(w.forge.merges) != 0 { + t.Fatalf("a warrant for another ask merged: %v", w.forge.merges) + } + d := w.delivery(IDOf("novox/app", readyHead)) + if !strings.Contains(d.Ready.Refused, "digest") || d.Ready.Approved != nil { + t.Fatalf("the refused answer is not kept: %+v", d.Ready) + } + if lines := w.h.Stalled(); len(lines) != 1 || !strings.Contains(lines[0].Says, "answer") { + t.Fatalf("a refused answer is not said loudly: %+v", lines) + } +} + +func TestADeclinedMergeIsNotMergedSaidOnceAndItsMarkTakenOff(t *testing.T) { + w := markedGreen(t) + w.op.answer(w.op.open()[0], OptionDecline) + for range 3 { + w.later(2 * time.Minute) + w.settleAll() + } + if len(w.forge.merges) != 0 || w.op.count() != 1 { + t.Fatalf("declined, yet merges %v and asks %d", w.forge.merges, w.op.count()) + } + if len(w.forge.comments) != 1 || !strings.Contains(w.forge.comments[0], "declined") || + !strings.Contains(w.forge.comments[0], "chose Decline") { + t.Fatalf("the Decline is not said once on the pull request, with its warrant: %v", w.forge.comments) + } + if len(w.forge.unlabels) == 0 || w.forge.unlabels[0] != "novox/app#7-31" { + t.Fatalf("the mark was not taken off: %v", w.forge.unlabels) + } +} + +func TestAPushAfterTheAskCancelsItAndTheNewHeadNeedsANewMarkAndApprove(t *testing.T) { + w := markedGreen(t) + old := w.op.open()[0] + w.h.PullUpdated(pr("novox/app", 7, pushed, "feat/x", "src/a.go")) + w.h.Checked(judgedAt("novox/app", 7, pushed, "pass", "t1t1t1t1t1t1")) + w.forge.open = []OpenPull{markedPull("novox/app", 7, pushed, readyHead)} // the old mark, on the new head + w.h.SeeOpenPulls() + w.settleAll() + if len(w.op.cancelled) != 1 || w.op.cancelled[0] != old || len(w.op.open()) != 0 { + t.Fatalf("the ask was not cancelled: cancelled %v, open %v", w.op.cancelled, w.op.open()) + } + if len(w.forge.merges) != 0 || w.op.count() != 1 { + t.Fatalf("merges %v, asks %d", w.forge.merges, w.op.count()) + } + if d := w.delivery(IDOf("novox/app", readyHead)); d.Ready.AskID != "" { + t.Fatalf("the cancelled ask is still kept as waiting: %+v", d.Ready) + } + // The new head, marked by its reviewer, is asked afresh. + w.forge.open = []OpenPull{markedPull("novox/app", 7, pushed, pushed)} + w.h.SeeOpenPulls() + w.settleAll() + if open := w.op.open(); len(open) != 1 || open[0] == old { + t.Fatalf("the new head was not asked afresh: %v", open) + } +} + +func TestAnExpiredAskIsAskedAgainOnce(t *testing.T) { + w := markedGreen(t) + first := w.op.open()[0] + w.op.end(first) + w.settleAll() + if w.op.count() != 2 || len(w.op.open()) != 1 || w.op.open()[0] == first { + t.Fatalf("an expired ask was not asked again once: %d asks, open %v", w.op.count(), w.op.open()) + } + for range 3 { + w.later(2 * time.Minute) + w.settleAll() + } + if w.op.count() != 2 || len(w.forge.merges) != 0 { + t.Fatalf("asked more than once again: %d asks, merges %v", w.op.count(), w.forge.merges) + } +} + +func TestAGroupIsAskedOnceForAllItsHeadsAndMergedInOrderOnOneApprove(t *testing.T) { + w := newWorld(t) + w.op.auto = false + const ctlHead, catHead = "c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1", "d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2" + ctl, cat := IDOf("novox/mesh-controller", ctlHead), IDOf("novox/mesh-catalog", catHead) + w.ctl.order = func(ms []Member) Order { return Order{Order: []string{ctl, cat}} } + w.h.PullUpdated(pr("novox/mesh-controller", 1, ctlHead, "feat/x", "internal/a.go")) + w.h.PullUpdated(pr("novox/mesh-catalog", 2, catHead, "feat/x", "modules/app/module.json")) + w.h.Checked(judgedAt("novox/mesh-controller", 1, ctlHead, "pass", "t1t1t1t1t1t1")) + w.h.Checked(judgedAt("novox/mesh-catalog", 2, catHead, "pass", "t3t3t3t3t3t3")) + w.forge.open = []OpenPull{markedPull("novox/mesh-controller", 1, ctlHead, ctlHead), + markedPull("novox/mesh-catalog", 2, catHead, catHead)} + w.h.SeeOpenPulls() + w.settleAll() + w.h.Checked(CheckedEvent{Group: "feat/x", ID: "check-1", Verdict: "pass", Summary: "both compose"}) + w.settleAll() + if open := w.op.open(); len(open) != 1 || len(w.forge.merges) != 0 { + t.Fatalf("a group is asked once before any merge: open %v, merges %v", w.op.open(), w.forge.merges) + } + a := w.op.ask(w.op.open()[0]) + if !strings.Contains(a.Headline, "group feat") || !strings.Contains(a.Details, "mesh-controller#1") || + !strings.Contains(a.Details, "mesh-catalog#2") { + t.Fatalf("the ask does not name the group and its members: %+v", a) + } + w.op.answer(a.ID, OptionApprove) + w.settleAll() + if len(w.forge.merges) != 1 || w.forge.merges[0] != "novox/mesh-controller#1@"+ctlHead { + t.Fatalf("the first member was not merged alone on the Approve: %v", w.forge.merges) + } + w.h.PullMerged(merged("novox/mesh-controller", 1, ctlHead, "m1m1m1m1m1m1")) + w.settleAll() + if len(w.forge.merges) != 2 || w.forge.merges[1] != "novox/mesh-catalog#2@"+catHead || w.op.count() != 1 { + t.Fatalf("the next member was not merged on the same Approve: merges %v, asks %d", w.forge.merges, w.op.count()) + } +} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go b/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go index c035e450..effcd900 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go @@ -561,6 +561,7 @@ type world struct { store *memStore ctl *fakeController forge *fakeForge + op *fakeOperator now time.Time said []map[string]any mu sync.Mutex @@ -584,6 +585,10 @@ func newWorldOver(t *testing.T, store *memStore, ctl *fakeController, forge *fak w.said = append(w.said, m) return nil }} + // The operator approves at once unless a test says otherwise (w.op.auto = false): most tests are not about the Approve. + w.op = newFakeOperator(w.h.Now) + w.op.auto = true + w.h.Operator = w.op w.h.init() if err := w.h.Load(); err != nil { t.Fatal(err) diff --git a/modules/mesh-delivery/cmd/mesh-delivery/holder.go b/modules/mesh-delivery/cmd/mesh-delivery/holder.go index 213e3b99..2d40a824 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/holder.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/holder.go @@ -27,6 +27,8 @@ type Holder struct { Emit func(event string, body any) error Now func() time.Time Logf func(format string, a ...any) + // Operator is the operator channel, asked for the Approve of a merge into main (approval.go). + Operator Operator mu sync.Mutex deliveries map[string]*Delivery @@ -381,7 +383,7 @@ func (h *Holder) freshAnnouncement(p PullEvent, now time.Time, startedOver strin d := &Delivery{ID: IDOf(p.repository(), p.HeadSHA), Repository: p.repository(), Commit: p.HeadSHA, Number: p.Number, Title: p.Title, Base: p.Base, Branch: p.Head, HTMLURL: p.HTMLURL, CloneURL: p.CloneURL, Paths: p.Paths, PathsTruncated: p.PathsTruncated, Removed: p.Removed, ModuleDirs: p.ModuleDirs, ModuleDirsSaid: p.ModuleDirsSaid, - After: AfterIn(p.Body), Created: now, BaseSHA: p.BaseSHA} + After: AfterIn(p.Body), Issues: IssuesIn(p.Title + "\n" + p.Body), Created: now, BaseSHA: p.BaseSHA} why := fmt.Sprintf("%s#%d's head announced", d.Repository, d.Number) if startedOver != "" { why = fmt.Sprintf("%s; %s", why, startedOver) @@ -551,7 +553,7 @@ func (h *Holder) PullMerged(p PullEvent) { d = &Delivery{ID: IDOf(p.repository(), head), Repository: p.repository(), Commit: head, Number: p.Number, Title: p.Title, Base: p.Base, Branch: p.Head, HTMLURL: p.HTMLURL, CloneURL: p.CloneURL, Paths: p.Paths, PathsTruncated: p.PathsTruncated, Removed: p.Removed, ModuleDirs: p.ModuleDirs, ModuleDirsSaid: p.ModuleDirsSaid, - After: AfterIn(p.Body), Created: now, FromForge: true} + After: AfterIn(p.Body), Issues: IssuesIn(p.Title + "\n" + p.Body), Created: now, FromForge: true} // Its check, as the forge holds it on the head: the verdict the pull request merged with. if s := p.HeadChecks["mesh/merge-gate"]; s != "" { d.Check = &Verdict{At: now, Gate: verdictOfState(s), Summary: "the head's mesh/merge-gate, as the forge holds it", @@ -1060,6 +1062,7 @@ func (h *Holder) Tick() { for _, ask := range h.leaveStopped() { ask() } + h.hearAsks() h.mu.Lock() asks := h.readyAsks(h.Now()) h.mu.Unlock() diff --git a/modules/mesh-delivery/cmd/mesh-delivery/main.go b/modules/mesh-delivery/cmd/mesh-delivery/main.go index 724b6b26..52fa7fa0 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/main.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/main.go @@ -16,6 +16,7 @@ import ( "time" stdio "git.novox.be/novox/mesh-sdk/go" + "git.novox.be/novox/mesh-sdk/go/asker" ) func logf(format string, a ...any) { fmt.Fprintf(os.Stderr, format+"\n", a...) } @@ -40,6 +41,7 @@ func main() { Forge: toolForge{ask: stdio.Ask}, Emit: func(event string, body any) error { return stdio.Emit(event, body) }, Logf: logf, + Operator: &asker.Client{Module: Module, State: stdio.State("asked")}, } h.init() l := &listening{now: "not yet: starting"} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/manifest_test.go b/modules/mesh-delivery/cmd/mesh-delivery/manifest_test.go index ade98c14..1915e3b2 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/manifest_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/manifest_test.go @@ -21,6 +21,7 @@ type manifest struct { Serves []string `json:"serves"` } `json:"claims"` Seats []any `json:"seats"` + Uses []string `json:"uses"` Consumes []string `json:"consumes"` Emits []string `json:"emits"` Invokes []string `json:"invokes"` @@ -56,7 +57,7 @@ func TestItClaimsTheDeliverySeatAndFollowsWhatItHandles(t *testing.T) { if !reflect.DeepEqual(m.Emits, []string{"transition", "group"}) { t.Fatalf("emits %v", m.Emits) } - if !reflect.DeepEqual(m.State, []string{"deliveries", "groups"}) { + if !reflect.DeepEqual(m.State, []string{"deliveries", "groups", "asked"}) { t.Fatalf("state %v", m.State) } // It calls exactly what its ports ask: the controller's six verbs and the forge's four tools. @@ -68,6 +69,11 @@ func TestItClaimsTheDeliverySeatAndFollowsWhatItHandles(t *testing.T) { "gitea.gitea_commit_statuses", "gitea.gitea_contains", "gitea.gitea_open_pulls", // The merge on a ready mark (novox/hq issue 722): the merge tool, a comment, a label taken off. "gitea.gitea_merge_pull_request", "gitea.gitea_add_comment", "gitea.gitea_api") + // A merge into main asks the operator for the Approve (novox/hq ADR 0398 §3, issue 948), as the knowledge base asks. + want = append(want, "seat:operator-channel.notify") + if !reflect.DeepEqual(m.Uses, []string{"operator-channel"}) { + t.Fatalf("uses %v", m.Uses) + } if !reflect.DeepEqual(m.Invokes, want) { t.Fatalf("invokes %v, wanted %v", m.Invokes, want) } diff --git a/modules/mesh-delivery/cmd/mesh-delivery/model.go b/modules/mesh-delivery/cmd/mesh-delivery/model.go index 432e079d..d4b4f68e 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/model.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/model.go @@ -35,6 +35,8 @@ type Delivery struct { ModuleDirsSaid bool `json:"module_dirs_said,omitempty"` // After are the repositories its pull request says it goes after (`after: ` lines). After []string `json:"after,omitempty"` + // Issues are the issues its title and body name ("issue 948"), shown to the operator who is asked to approve its merge. + Issues []int `json:"issues,omitempty"` // Group is the delivery group it is in: its head branch name. Empty for a lone delivery. Group string `json:"group,omitempty"` diff --git a/modules/mesh-delivery/cmd/mesh-delivery/operator_test.go b/modules/mesh-delivery/cmd/mesh-delivery/operator_test.go new file mode 100644 index 00000000..6c312b21 --- /dev/null +++ b/modules/mesh-delivery/cmd/mesh-delivery/operator_test.go @@ -0,0 +1,104 @@ +package main + +import ( + "sync" + "time" + + "git.novox.be/novox/mesh-sdk/go/asks" +) + +// fakeOperator is the operator channel: it keeps the asks made, and answers as the test says. In auto mode it +// approves every ask the moment it is read (the tests of other behaviour, which are not about the Approve). +type fakeOperator struct { + mu sync.Mutex + now func() time.Time + auto bool + asked []asks.Ask + cancelled []string + state map[string]string + warrant map[string]*asks.Warrant + refuse error +} + +func newFakeOperator(now func() time.Time) *fakeOperator { + return &fakeOperator{now: now, state: map[string]string{}, warrant: map[string]*asks.Warrant{}} +} + +func (o *fakeOperator) Ask(a asks.Ask) error { + o.mu.Lock() + defer o.mu.Unlock() + if o.refuse != nil { + return o.refuse + } + if err := a.Check(o.now()); err != nil { + return err + } + o.asked = append(o.asked, a) + o.state[a.ID] = "open" + return nil +} + +func (o *fakeOperator) Cancel(id string) error { + o.mu.Lock() + defer o.mu.Unlock() + o.cancelled = append(o.cancelled, id) + o.state[id] = "cancelled" + return nil +} + +func (o *fakeOperator) Where(id string) (string, *asks.Warrant, error) { + o.mu.Lock() + defer o.mu.Unlock() + if o.auto && o.state[id] == "open" { + o.answerLocked(id, OptionApprove) + } + return o.state[id], o.warrant[id], nil +} + +func (o *fakeOperator) ask(id string) asks.Ask { + for _, a := range o.asked { + if a.ID == id { + return a + } + } + panic("no ask " + id) +} + +func (o *fakeOperator) answerLocked(id, option string) { + a := o.ask(id) + opt, _ := a.Option(option) + o.state[id] = "answered" + o.warrant[id] = &asks.Warrant{Ask: id, Asker: Module, Outcome: asks.OutcomeChosen, Option: option, Label: opt.Label, + Level: opt.Level, By: &asks.Person{Who: asks.Operator, Kind: "telegram", Verified: "user id verified"}, + At: o.now(), AskDigest: a.Digest()} +} + +// answer is the operator's choice, proven by the router. +func (o *fakeOperator) answer(id, option string) { + o.mu.Lock() + defer o.mu.Unlock() + o.answerLocked(id, option) +} + +// end is an ask that closed without a choice. +func (o *fakeOperator) end(id string) { + o.mu.Lock() + defer o.mu.Unlock() + o.state[id] = "expired" + o.warrant[id] = &asks.Warrant{Ask: id, Asker: Module, Outcome: asks.OutcomeExpired, At: o.now()} +} + +// open is the ids of the asks waiting for an answer, in the order asked. +func (o *fakeOperator) open() []string { + o.mu.Lock() + defer o.mu.Unlock() + var out []string + for _, a := range o.asked { + if o.state[a.ID] == "open" { + out = append(out, a.ID) + } + } + return out +} + +func (o *fakeOperator) count() int { o.mu.Lock(); defer o.mu.Unlock(); return len(o.asked) } diff --git a/modules/mesh-delivery/cmd/mesh-delivery/ready.go b/modules/mesh-delivery/cmd/mesh-delivery/ready.go index fa36708a..ca66438b 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/ready.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/ready.go @@ -4,6 +4,8 @@ import ( "fmt" "strings" "time" + + "git.novox.be/novox/mesh-sdk/go/asks" ) // Merge on ready (novox/hq issue 722). @@ -72,6 +74,21 @@ type ReadyRun struct { // OtherBase is the branch its pull request merges into when that is not main: never merged here, said once on the // pull request (novox/hq ADR 0398 §4, issue 948). OtherBase string `json:"other_base,omitempty"` + + // The operator's Approve of the merge into main (novox/hq ADR 0398 §3, issue 948, approval.go). AskID is the ask + // waiting for an answer, Ask the ask as made (a warrant is held to it), Heads the heads it covers by delivery id (a + // group's members together), AskedAt when and Asks how many were made for this head. + AskID string `json:"ask_id,omitempty"` + Ask *asks.Ask `json:"ask,omitempty"` + Heads map[string]string `json:"heads,omitempty"` + AskedAt time.Time `json:"asked_at"` + Asks int `json:"asks,omitempty"` + // Approved and Declined are when the operator chose, Warrant what the router's warrant says of who and how; Refused + // why an answer was not taken (it did not hold to the ask). + Approved *time.Time `json:"approved,omitempty"` + Declined *time.Time `json:"declined,omitempty"` + Warrant string `json:"warrant,omitempty"` + Refused string `json:"refused,omitempty"` } // MergeBranch is the one branch this owner merges into (novox/hq ADR 0398 §4): a merge into any other is its author's. @@ -86,6 +103,9 @@ func (r *ReadyRun) mergedBy() string { if r.By != "" { by += " by " + r.By } + if r.Warrant != "" { + by += "; the merge into main approved: " + r.Warrant + } return by } @@ -156,6 +176,9 @@ func (h *Holder) readyAsks(now time.Time) []func() { continue } m.cleared = now + if old := h.deliveries[IDOf(repository, mark.Head)]; old != nil && old.Ready != nil && old.Ready.AskID != "" { + asks = append(asks, h.dropAsk(old.Ready.AskID, "its head moved on to "+shortOf(p.HeadSHA, 12))) + } asks = append(asks, h.clearMark(k, p, mark)) continue } @@ -163,7 +186,7 @@ func (h *Holder) readyAsks(now time.Time) []func() { if d == nil || d.State.Final() || d.MergedAs != "" { continue // not announced yet, or past its merge } - if d.Ready == nil || d.Ready.Head != mark.Head { + if d.Ready == nil || d.Ready.Head != mark.Head || (d.Ready.Declined != nil && mark.At != d.Ready.MarkedAt) { d.Ready = &ReadyRun{Head: mark.Head, By: mark.By, MarkedAt: mark.At, Seen: now} h.Logf("[mesh-delivery] %s is marked ready at %s by %s: merged once its check passes against its trunk's head", d.ID, shortOf(mark.Head, 12), orNone(mark.By)) @@ -173,6 +196,14 @@ func (h *Holder) readyAsks(now time.Time) []func() { if r.Merged != nil { continue } + if r.Declined != nil { + // The operator declined: nothing is merged, and the mark is taken off (said once when it was declined). + if m.cleared.IsZero() || now.Sub(m.cleared) >= readyTryEvery { + m.cleared = now + asks = append(asks, h.unmark(k, p, mark)) + } + continue + } if d.State == Rejected { if r.Failed == "" { r.Failed = "its check failed" @@ -212,6 +243,17 @@ func (h *Holder) readyAsks(now time.Time) []func() { } h.keep(d) } + var approvalAsk func() + if waits == "" && p.Base == MergeBranch { + waits, approvalAsk = h.approval(d, now) + } + if waits != r.Waits { + r.Waits = waits + h.keep(d) + } + if approvalAsk != nil { + asks = append(asks, approvalAsk) + } if waits != "" || (!r.LastTry.IsZero() && now.Sub(r.LastTry) < readyTryEvery) { continue } @@ -328,6 +370,19 @@ func (h *Holder) clearMark(k string, p OpenPull, mark ReadyMark) func() { } } +// unmark takes the mark off a pull request whose merge the operator declined (already said on it). +func (h *Holder) unmark(k string, p OpenPull, mark ReadyMark) func() { + return func() { + if err := h.Forge.Unlabel(p.Owner, p.Repo, p.Number, mark.LabelID); err != nil { + h.Logf("[mesh-delivery] %s's ready mark could not be taken off after the operator's Decline yet: %v", k, err) + return + } + h.mu.Lock() + delete(h.marks, k) + h.mu.Unlock() + } +} + // clearedSaying is what a cleared mark says on its pull request. A mark with no head is a label no "ready at " // comment of a listed account followed (novox/hq issue 873); one with a head names an older head than the pull's. func clearedSaying(mark ReadyMark, p OpenPull) string { @@ -365,6 +420,11 @@ func (h *Holder) readyLines(now time.Time) []StalledLine { Bound: "0s", H2: "none: a new head marked ready, or a recheck that passes — the operator's", Says: fmt.Sprintf("%s#%d is marked ready at %s and not merged: %s", d.Repository, d.Number, shortOf(r.Head, 12), r.Failed)}) + case r.Refused != "": + out = append(out, StalledLine{ID: d.ID, Number: d.Number, State: d.State, For: now.Sub(r.Seen).Round(time.Second).String(), + Bound: "0s", H2: "none: a new head marked ready, which asks the operator again — the operator's", + Says: fmt.Sprintf("%s#%d is marked ready at %s and not merged: the operator's answer %s", d.Repository, + d.Number, shortOf(r.Head, 12), r.Refused)}) case r.RefusedSince != nil && now.Sub(*r.RefusedSince) >= readyRefusedFor: out = append(out, StalledLine{ID: d.ID, Number: d.Number, State: d.State, For: now.Sub(*r.RefusedSince).Round(time.Second).String(), Bound: readyRefusedFor.String(), diff --git a/modules/mesh-delivery/go.mod b/modules/mesh-delivery/go.mod index 9ce486e4..8d6748fb 100644 --- a/modules/mesh-delivery/go.mod +++ b/modules/mesh-delivery/go.mod @@ -2,4 +2,4 @@ module mesh-delivery go 1.22 -require git.novox.be/novox/mesh-sdk/go v0.1.7 +require git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261013182924-5933eb757d44 diff --git a/modules/mesh-delivery/go.sum b/modules/mesh-delivery/go.sum index b474419e..3c290b47 100644 --- a/modules/mesh-delivery/go.sum +++ b/modules/mesh-delivery/go.sum @@ -1,2 +1,2 @@ -git.novox.be/novox/mesh-sdk/go v0.1.7 h1:C0sTQmtTiyYH7bnqZb7PusXnqA37gKuT7Nqjn9gG47w= -git.novox.be/novox/mesh-sdk/go v0.1.7/go.mod h1:GFuZUElBZ9A++mxgIKo97aXXo+kV0uJ/UkbhQPPIbrY= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261013182924-5933eb757d44/go.mod h1:GFuZUElBZ9A++mxgIKo97aXXo+kV0uJ/UkbhQPPIbrY= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261013182924-5933eb757d44 h1:r5xIo5qWOXQdCSMxvqJy8kV4U+P/QWXFK6971Ha6wU8= diff --git a/modules/mesh-delivery/module.json b/modules/mesh-delivery/module.json index b32b6320..f951f918 100644 --- a/modules/mesh-delivery/module.json +++ b/modules/mesh-delivery/module.json @@ -51,11 +51,16 @@ "gitea.gitea_open_pulls", "gitea.gitea_merge_pull_request", "gitea.gitea_add_comment", - "gitea.gitea_api" + "gitea.gitea_api", + "seat:operator-channel.notify" + ], + "uses": [ + "operator-channel" ], "state": [ "deliveries", - "groups" + "groups", + "asked" ], "tools": [ "delivery_status"