Merge main (#514, issue 1733) into the delivery-records group
This commit is contained in:
@@ -148,8 +148,32 @@ func touchesSaid(d *Delivery) string {
|
||||
return strings.Join(d.Check.Modules, ", ")
|
||||
}
|
||||
|
||||
// checkClause is the check's state as the ask's summary says it after the pull request's or the group's name: sep and
|
||||
// "passed its check", or ": its check has no verdict yet: it is running (since 21:05 UTC)" and the like.
|
||||
func checkClause(c checkSaid, sep string) string {
|
||||
if c.passed() {
|
||||
return sep + c.Words
|
||||
}
|
||||
return ": " + c.Words
|
||||
}
|
||||
|
||||
// approveCaveat is what an Approve does beyond the merge while the check has not passed: it merges before the check ends,
|
||||
// or although it failed (the check gates no merge, check-gates-merge false). Empty once it passed.
|
||||
func approveCaveat(c checkSaid) string {
|
||||
switch c.Key {
|
||||
case checkSaidPass:
|
||||
return ""
|
||||
case checkSaidFail:
|
||||
return " although its check failed"
|
||||
}
|
||||
return " now, before its check ends"
|
||||
}
|
||||
|
||||
// composeMergeAsk is the ask for a delivery, or for a group's members together.
|
||||
func composeMergeAsk(id string, d *Delivery, group string, members []*Delivery, heads map[string]string, now time.Time) asks.Ask {
|
||||
// check is the state of the check at the heads it covers (checkAt): the ask says "passed its check" only when it did, and
|
||||
// otherwise that Approve merges before the check ends, or although it failed (issue 1733).
|
||||
func composeMergeAsk(id string, d *Delivery, group string, members []*Delivery, heads map[string]string, check checkSaid,
|
||||
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"
|
||||
@@ -170,8 +194,8 @@ func composeMergeAsk(id string, d *Delivery, group string, members []*Delivery,
|
||||
}
|
||||
if group == "" {
|
||||
a.Headline = phone("Merge into main: "+pullSaid(d)+" "+d.Title, asks.HeadlineLength)
|
||||
summary := fmt.Sprintf("%s %s passed its checks. Approve merges it into main at %s; Decline leaves it unmerged.",
|
||||
pullSaid(d), d.Title, shortOf(d.Commit, 12))
|
||||
summary := fmt.Sprintf("%s %s%s. Approve merges it into main at %s%s; Decline leaves it unmerged.",
|
||||
pullSaid(d), d.Title, checkClause(check, " "), shortOf(d.Commit, 12), approveCaveat(check))
|
||||
if unreviewed != "" {
|
||||
summary = strings.ToUpper(unreviewed[:1]) + unreviewed[1:] + ". " + summary
|
||||
}
|
||||
@@ -187,8 +211,12 @@ func composeMergeAsk(id string, d *Delivery, group string, members []*Delivery,
|
||||
names = append(names, pullSaid(m))
|
||||
}
|
||||
a.Headline = phone("Merge into main: group "+group+" ("+strings.Join(names, ", ")+")", asks.HeadlineLength)
|
||||
summary := fmt.Sprintf("The group %s, %d pull requests, passed its checks. Approve merges all of them into main in one pass, at the heads named; Decline leaves them unmerged.",
|
||||
group, len(members))
|
||||
caveat := approveCaveat(check)
|
||||
if caveat != "" {
|
||||
caveat = "," + caveat
|
||||
}
|
||||
summary := fmt.Sprintf("The group %s, %d pull requests%s. Approve merges all of them into main in one pass, at the heads named%s; Decline leaves them unmerged.",
|
||||
group, len(members), checkClause(check, ", "), caveat)
|
||||
if unreviewed != "" {
|
||||
summary = strings.ToUpper(unreviewed[:1]) + unreviewed[1:] + ". " + summary
|
||||
}
|
||||
@@ -310,6 +338,10 @@ func (h *Holder) endStaleAsks() {
|
||||
h.cancels = append(h.cancels, h.dropAsk(r.AskID, "its head "+shortOf(d.Commit, 12)+" is no longer the pull request's head, or the pull request closed"))
|
||||
case !h.headsHold(d, r):
|
||||
h.cancels = append(h.cancels, h.dropAsk(r.AskID, "a head it covers moved on: the heads it covers are no longer the group's heads"))
|
||||
case r.Approved == nil && r.AskID != "" && r.AskCheck != "" && !h.checkGates() && h.checkMoved(d, r.AskCheck):
|
||||
// The check gates no merge, so the ask was made before its verdict: once the check passes or fails (or a pass it
|
||||
// named is gone), the ask is asked again in words that say so (issue 1733). Running and not started say the same.
|
||||
h.cancels = append(h.cancels, h.withdrawAsk(r.AskID, "the check at its heads is now: "+h.checkAt(h.askMembers(d)).Words))
|
||||
case r.Approved == nil && r.AskID != "" && !r.AskedAt.IsZero() && !h.Now().Before(r.AskedAt.Add(AskLasts)):
|
||||
// The ask lapsed at the router with nobody's answer, and nothing in the gate's inputs says so: it is taken away and
|
||||
// written again, which the teller asks afresh (ADR 0441 §1).
|
||||
@@ -318,6 +350,16 @@ func (h *Holder) endStaleAsks() {
|
||||
}
|
||||
}
|
||||
|
||||
// checkMoved is whether the check at the heads of d's ask reached a verdict its pending ask does not say: it passed or failed
|
||||
// since, or the pass it said is gone. Under the lock.
|
||||
func (h *Holder) checkMoved(d *Delivery, said string) bool {
|
||||
now := h.checkAt(h.askMembers(d)).Key
|
||||
if now == said {
|
||||
return false
|
||||
}
|
||||
return now == checkSaidPass || now == checkSaidFail || said == checkSaidPass || said == checkSaidFail
|
||||
}
|
||||
|
||||
// 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()) {
|
||||
@@ -368,19 +410,20 @@ func (h *Holder) approval(d *Delivery, now time.Time) (string, func()) {
|
||||
asking = max(asking, m.Ready.Asking+1)
|
||||
}
|
||||
}
|
||||
ask := composeMergeAsk(sdk.AskingID(id, asking), d, group, members, heads, now)
|
||||
check := h.checkAt(members)
|
||||
ask := composeMergeAsk(sdk.AskingID(id, asking), d, group, members, heads, check, now)
|
||||
taken := []*Delivery{}
|
||||
var reads []memberRead
|
||||
for _, m := range members {
|
||||
h.runOf(m, now)
|
||||
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)
|
||||
m.Ready.AskedKey, m.Ready.Asking = id, asking
|
||||
m.Ready.AskedKey, m.Ready.Asking, m.Ready.AskCheck = id, asking, check.Key
|
||||
h.keep(m)
|
||||
taken = append(taken, m)
|
||||
reads = append(reads, readOf(m, m.Commit))
|
||||
}
|
||||
h.Logf("[mesh-delivery] %s passed its checks: the operator is asked to Approve its merge into main (%s, ask %d)",
|
||||
d.ID, id, r.Asks)
|
||||
h.Logf("[mesh-delivery] %s (%s): the operator is asked to Approve its merge into main (%s, ask %d)",
|
||||
d.ID, check.Words, id, r.Asks)
|
||||
gid := ""
|
||||
if group != "" {
|
||||
gid = d.Group
|
||||
@@ -433,7 +476,7 @@ func (h *Holder) approval(d *Delivery, now time.Time) (string, func()) {
|
||||
h.mu.Lock()
|
||||
defer h.mu.Unlock()
|
||||
undo()
|
||||
h.Logf("[mesh-delivery] %s passed its checks and the operator is not asked: %s", d.ID, says)
|
||||
h.Logf("[mesh-delivery] %s (%s): the operator is not asked: %s", d.ID, check.Words, says)
|
||||
h.lookAt(h.Now(), "an ask not made: the forge account cannot write a repository it covers")
|
||||
return
|
||||
}
|
||||
|
||||
@@ -287,7 +287,7 @@ func (h *Holder) ForgeMerged(f sdk.Forge) {
|
||||
r, now := d.Ready, h.Now()
|
||||
d.Owed = append(d.Owed, Effect{Kind: EffectEmit, Event: "merged", Line: string(mustJSON(mergedEvent(d, f.Head))), Since: now})
|
||||
r.Merged, r.Answer, r.RefusedSince, r.merging, r.MergeCommit = &now, "merged", nil, false, f.Merge
|
||||
h.Logf("[mesh-delivery] %s merged on the operator's Approve at %s, its check passed against its trunk's head", d.ID, shortOf(f.Head, 12))
|
||||
h.Logf("[mesh-delivery] %s merged on the operator's Approve at %s (%s)", d.ID, shortOf(f.Head, 12), h.checkAt(h.askMembers(d)).Words)
|
||||
h.keep(d)
|
||||
}
|
||||
|
||||
|
||||
@@ -302,3 +302,72 @@ func checkerRefusal(id string) error {
|
||||
"its trunk or its group, and nothing is asked of it here (novox/hq ADR 0447). Read where its check stands with the "+
|
||||
"checker's checker_pull tool", id)
|
||||
}
|
||||
|
||||
// The states of the check at an ask's heads, as the ask says them (issue 1733).
|
||||
const (
|
||||
checkSaidPass = "pass"
|
||||
checkSaidRunning = "running"
|
||||
checkSaidNone = "none"
|
||||
checkSaidFail = "fail"
|
||||
)
|
||||
|
||||
// checkSaid is the state of the check at the heads an ask covers, and its words for the ask: what the checker's verdict at
|
||||
// exactly those heads says, never what the gate would like it to say (issue 1733). Key is one of the checkSaid states.
|
||||
type checkSaid struct {
|
||||
Key string
|
||||
Words string
|
||||
}
|
||||
|
||||
// passed is whether the check at the heads passed.
|
||||
func (c checkSaid) passed() bool { return c.Key == checkSaidPass }
|
||||
|
||||
// checkAt reads the check at these members' heads: the checker's verdict for exactly those heads (its verdicts bucket,
|
||||
// keyed by their hash), and, while none is held, the forge's mesh/check at a member's head (a failure or a pending one
|
||||
// there). "passed its check" is said only for a passing verdict at these heads. Under the lock.
|
||||
func (h *Holder) checkAt(members []*Delivery) checkSaid {
|
||||
v, hash, ok := h.checkerAt(members)
|
||||
if ok {
|
||||
switch v.State {
|
||||
case checkerPass:
|
||||
return checkSaid{Key: checkSaidPass, Words: "passed its check"}
|
||||
case checkerRunning:
|
||||
words := "its check has no verdict yet: it is running"
|
||||
if since, held := h.runningSince[hash]; held && !since.IsZero() {
|
||||
words += " (since " + since.UTC().Format("15:04") + " UTC)"
|
||||
}
|
||||
return checkSaid{Key: checkSaidRunning, Words: words}
|
||||
default:
|
||||
why := firstOf(oneLine(v.Failed.Test), oneLine(v.Failed.Words), v.State)
|
||||
if v.Failed.Test != "" && v.Failed.Words != "" {
|
||||
why = oneLine(v.Failed.Test) + ": " + oneLine(v.Failed.Words)
|
||||
}
|
||||
return checkSaid{Key: checkSaidFail, Words: "its check failed: " + clip(why, 120)}
|
||||
}
|
||||
}
|
||||
for _, m := range members {
|
||||
switch m.CheckState {
|
||||
case "failure", "error":
|
||||
why := "mesh/check is " + m.CheckState + " at " + shortOf(m.Commit, 12)
|
||||
if m.Check != nil && m.Check.Summary != "" {
|
||||
why = oneLine(m.Check.Summary)
|
||||
}
|
||||
return checkSaid{Key: checkSaidFail, Words: "its check failed: " + clip(why, 120)}
|
||||
}
|
||||
}
|
||||
succeeded := 0
|
||||
for _, m := range members {
|
||||
switch m.CheckState {
|
||||
case "pending":
|
||||
return checkSaid{Key: checkSaidRunning, Words: "its check has no verdict yet: it is running"}
|
||||
case "success":
|
||||
succeeded++
|
||||
}
|
||||
}
|
||||
switch {
|
||||
case succeeded > 0 && len(members) == 1:
|
||||
return checkSaid{Key: checkSaidPass, Words: "passed its check"} // its head's mesh/check is the checker's verdict at it
|
||||
case succeeded == len(members) && succeeded > 0:
|
||||
return checkSaid{Key: checkSaidNone, Words: "its members' own checks passed; the check of their heads together has not answered"}
|
||||
}
|
||||
return checkSaid{Key: checkSaidNone, Words: "its check has no verdict yet"}
|
||||
}
|
||||
|
||||
@@ -115,6 +115,10 @@ type ReadyRun struct {
|
||||
Heads map[string]string `json:"heads,omitempty"`
|
||||
AskedAt time.Time `json:"asked_at"`
|
||||
Asks int `json:"asks,omitempty"`
|
||||
// AskCheck is the state of the check at the heads the pending ask names, as its words said it (checkAt: pass, running,
|
||||
// none or fail; issue 1733): while the check gates no merge, an ask whose check reached a verdict since is asked again
|
||||
// in new words, so it never keeps a claim the check no longer holds.
|
||||
AskCheck string `json:"ask_check,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"`
|
||||
@@ -186,7 +190,8 @@ func (h *Holder) readyAsks(now time.Time) []func() {
|
||||
if d.Ready == nil && d.State == Ready {
|
||||
h.runOf(d, now)
|
||||
h.Poke() // its check, if none is asked, is worth asking now (askCheck)
|
||||
h.Logf("[mesh-delivery] %s: its checks passed; the operator's Approve of its merge into main is asked once it may merge", d.ID)
|
||||
h.Logf("[mesh-delivery] %s is ready (%s); the operator's Approve of its merge into main is asked once it may merge",
|
||||
d.ID, h.checkAt([]*Delivery{d}).Words)
|
||||
}
|
||||
if a := h.sayAfterLine(d); a != nil {
|
||||
asks = append(asks, a)
|
||||
@@ -227,11 +232,12 @@ func (h *Holder) readyAsks(now time.Time) []func() {
|
||||
if d.Base != MergeBranch {
|
||||
if r.OtherBase != d.Base {
|
||||
r.OtherBase = d.Base
|
||||
h.Logf("[mesh-delivery] %s passed its checks and is not asked: it merges into %s, not %s", d.ID, d.Base, MergeBranch)
|
||||
check := h.checkAt([]*Delivery{d}).Words // ready need not mean passed: the check may gate no merge (issue 1733)
|
||||
h.Logf("[mesh-delivery] %s (%s) is not asked: it merges into %s, not %s", d.ID, check, d.Base, MergeBranch)
|
||||
h.keep(d)
|
||||
asks = append(asks, h.sayOn(d, fmt.Sprintf("mesh-delivery: **not merged.** This pull request's checks passed at "+
|
||||
"%s, and it merges into %s. Only a merge into %s is this owner's; a merge into another branch is its "+
|
||||
"author's (novox/hq ADR 0398 §4).", d.Commit, d.Base, MergeBranch)))
|
||||
asks = append(asks, h.sayOn(d, fmt.Sprintf("mesh-delivery: **not merged.** This pull request is at %s (%s), "+
|
||||
"and it merges into %s. Only a merge into %s is this owner's; a merge into another branch is its "+
|
||||
"author's (novox/hq ADR 0398 §4).", d.Commit, check, d.Base, MergeBranch)))
|
||||
}
|
||||
continue
|
||||
}
|
||||
@@ -524,13 +530,14 @@ func (h *Holder) readyLines(now time.Time) []StalledLine {
|
||||
case d.Base == "" && d.Number != 0 && now.Sub(r.Seen) >= headHeardBound:
|
||||
out = append(out, StalledLine{ID: d.ID, Number: d.Number, State: d.State, For: now.Sub(r.Seen).Round(time.Second).String(),
|
||||
Bound: headHeardBound.String(), H2: "none: a later announcement of the pull request's head (the merge tool announces it again, the forge's announcer restarts, or a new push) or an edit of its description; a recheck does not fill it — the operator's",
|
||||
Says: fmt.Sprintf("%s#%d passed its checks at %s and waits for %s", d.Repository, d.Number, shortOf(r.Head, 12), waitsForHead)})
|
||||
Says: fmt.Sprintf("%s#%d at %s (%s) waits for %s", d.Repository, d.Number, shortOf(r.Head, 12),
|
||||
h.checkAt([]*Delivery{d}).Words, waitsForHead)})
|
||||
case r.CannotWrite != "" && r.Approved == nil && r.CannotWriteSince != nil:
|
||||
out = append(out, StalledLine{ID: d.ID, Number: d.Number, State: d.State,
|
||||
For: now.Sub(*r.CannotWriteSince).Round(time.Second).String(), Bound: "0s",
|
||||
H2: "none: the forge account given write on the repository (gitea_grant_write; the gitea module grants it itself once a module of it is registered), which asks the operator then",
|
||||
Says: fmt.Sprintf("%s#%d passed its checks at %s and the operator is not asked for its Approve: %s",
|
||||
d.Repository, d.Number, shortOf(r.Head, 12), r.CannotWrite)})
|
||||
Says: fmt.Sprintf("%s#%d at %s (%s): the operator is not asked for its Approve: %s",
|
||||
d.Repository, d.Number, shortOf(r.Head, 12), h.checkAt([]*Delivery{d}).Words, r.CannotWrite)})
|
||||
case r.Failed != "" && d.State == Rejected:
|
||||
// Its cure is its author's new head: listed here at once (`show`), told on the pull request at once, and the
|
||||
// operator's only once no new head came within the author's bound (novox/hq ADR 0505 §5).
|
||||
|
||||
@@ -1,11 +1,15 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"sort"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"git.novox.be/novox/mesh-sdk/go/asks"
|
||||
sdk "git.novox.be/novox/mesh-sdk/go/delivery"
|
||||
)
|
||||
|
||||
// **A pull request whose checks passed and whose merge the operator approved is merged by this owner** (novox/hq issues
|
||||
@@ -65,6 +69,73 @@ func TestAnApprovedPullRequestIsMergedThroughTheMergeTool(t *testing.T) {
|
||||
t.Fatalf("merged again: %v", w.forge.merges)
|
||||
}
|
||||
|
||||
// 1733: the Approve's ask says the check's state at the heads it names, read from the checker's verdict there: "passed
|
||||
// its check" only when it passed. While the check gates no merge, the ask is made before any verdict, says that Approve
|
||||
// merges before the check ends, and is asked again in new words once the check passes or fails; a running check is no
|
||||
// new word.
|
||||
for _, row := range []struct {
|
||||
name, state, failed string
|
||||
gated bool
|
||||
asks int
|
||||
want, not []string
|
||||
}{
|
||||
{name: "gated, passed", gated: true, state: "pass", asks: 1,
|
||||
want: []string{"app#7 Fix it passed its check. Approve merges it into main at " + readyHead[:12] + ";"},
|
||||
not: []string{"before its check ends", "no verdict yet"}},
|
||||
{name: "ungated, no verdict", asks: 1,
|
||||
want: []string{"its check has no verdict yet", "Approve merges it into main at " + readyHead[:12] + " now, before its check ends"},
|
||||
not: []string{"passed"}},
|
||||
{name: "ungated, running", state: "running", asks: 1,
|
||||
want: []string{"its check has no verdict yet", "before its check ends"}, not: []string{"passed"}},
|
||||
{name: "ungated, then passed", state: "pass", asks: 2,
|
||||
want: []string{"passed its check. Approve merges it into main at " + readyHead[:12] + ";"}, not: []string{"before its check ends"}},
|
||||
{name: "ungated, then failed", state: "fail", failed: "TestThing: wanted 1, got 2", asks: 2,
|
||||
want: []string{"its check failed: TestThing: wanted 1, got 2", "although its check failed"}, not: []string{"passed"}},
|
||||
} {
|
||||
t.Run(row.name, func(t *testing.T) {
|
||||
v := newWorld(t)
|
||||
if !row.gated {
|
||||
ungated(v)
|
||||
}
|
||||
v.op.auto = false
|
||||
p := pr("novox/app", 7, readyHead, "feat/x", "src/a.go")
|
||||
p.Title = "Fix it"
|
||||
v.h.PullUpdated(p)
|
||||
put := func() {
|
||||
hash, _ := sdk.HeadsHash([]sdk.Head{headOf("novox/app", 7, readyHead)})
|
||||
value, _ := json.Marshal(map[string]any{"v": 1, "group": "g", "heads": hash, "state": row.state,
|
||||
"failed": map[string]string{"words": row.failed}})
|
||||
v.h.CheckerVerdictChanged("g."+hash, value)
|
||||
}
|
||||
if row.gated {
|
||||
v.h.Checked(judgedAt("novox/app", 7, readyHead, "pass", "t1t1t1t1t1t1"))
|
||||
}
|
||||
v.settleAll()
|
||||
if !row.gated && row.state != "" {
|
||||
put()
|
||||
v.settleAll()
|
||||
}
|
||||
open := v.op.open()
|
||||
if len(open) != 1 || v.op.count() != row.asks {
|
||||
t.Fatalf("open asks %v, asked %d times; wanted one open, asked %d times", open, v.op.count(), row.asks)
|
||||
}
|
||||
e := v.op.ask(open[0]).Explanation
|
||||
for _, w := range row.want {
|
||||
if !strings.Contains(e, w) {
|
||||
t.Fatalf("the ask does not say %q:\n%s", w, e)
|
||||
}
|
||||
}
|
||||
for _, w := range row.not {
|
||||
if strings.Contains(e, w) {
|
||||
t.Fatalf("the ask says %q:\n%s", w, e)
|
||||
}
|
||||
}
|
||||
if err := asks.Glance(e); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// check-gates-merge false (the operator, 2026-10-25 22:56): a reviewed pull request is asked with no verdict, another
|
||||
// merge moving its trunk puts it out of nothing, a failed verdict is shown on it and holds nothing, and the gate's
|
||||
// status and show say the mode.
|
||||
@@ -255,6 +326,11 @@ func TestAGroupMergesEveryMemberInOnePassOnlyOnceComposedAndApproved(t *testing.
|
||||
if u.op.count() != 1 {
|
||||
t.Fatalf("ungated, the group is asked %d times; wanted once", u.op.count())
|
||||
}
|
||||
// 1733: the group's ask claims no pass it has not had.
|
||||
if e := u.op.ask(u.op.open()[0]).Explanation; strings.Contains(e, "passed") ||
|
||||
!strings.Contains(e, "2 pull requests: its check has no verdict yet. Approve merges all of them into main in one pass, at the heads named, now, before its check ends;") {
|
||||
t.Fatalf("ungated, the group's ask does not say its check has no verdict yet:\n%s", e)
|
||||
}
|
||||
u.op.answer(u.op.open()[0], OptionApprove)
|
||||
u.settleAll()
|
||||
if len(u.forge.merges) != 2 {
|
||||
|
||||
Reference in New Issue
Block a user