mesh-issues: a fix whose pull request has no record learns its merge from the delivery or the forge, so a delivered event can cover it (issue 1950)

This commit is contained in:
2026-10-27 01:24:57 +01:00
parent 64530a8dcb
commit b51f03c4ff
8 changed files with 402 additions and 67 deletions
+36 -31
View File
@@ -18,6 +18,7 @@ package main
import (
"fmt"
"sort"
"strings"
"time"
)
@@ -112,45 +113,38 @@ func (t *Tracker) clearNotResolved(is *Issue) error {
// askUnheard asks mesh-delivery for a few fixes of issues not yet resolved that the tracker never heard of: a pull
// request linked after its delivery moved hears no transition again, and OnMoved asks only at a move.
func (t *Tracker) askUnheard() error {
if t.Deliveries == nil {
if t.Deliveries == nil && t.Pull == nil {
return nil
}
all, err := t.allIssues()
if err != nil {
return err
}
asked := 0
for n, is := range all {
if !unresolved(is) {
continue
// Issues not resolved first, then the others: a resolved issue's fix merged before the tracker heard deliveries is learned
// too, so its delivery line stops saying nothing was heard (issue 1950). The asks are bounded; the rest wait for the next sweep.
numbers := make([]uint64, 0, len(all))
for n := range all {
numbers = append(numbers, n)
}
sort.Slice(numbers, func(i, j int) bool {
a, b := unresolved(all[numbers[i]]), unresolved(all[numbers[j]])
if a != b {
return a
}
heard := false
for _, pr := range fixPullRequests(is) {
if fixOf(is, pr) != nil {
continue
}
if asked == MostSweepAsks {
return nil
}
asked++
repo, number, _ := strings.Cut(pr, "#")
d, err := t.Deliveries(repo, number)
if err != nil {
return fmt.Errorf("the delivery of %s (issue %d): %v", pr, n, err)
}
if d.State == "" {
continue
}
since := d.Since
if since.IsZero() {
since = t.now()
}
if _, err := t.recordFix(pr, d.heard(since), n); err != nil {
return err
}
heard = true
return numbers[i] < numbers[j]
})
left := MostSweepAsks
for _, n := range numbers {
if left == 0 {
return nil
}
if heard {
is := all[n]
asked, kept, err := t.learnUnheard(n, is, left)
left -= asked
if err != nil {
return err
}
if kept {
if err := t.afterFix(n); err != nil {
return err
}
@@ -159,6 +153,17 @@ func (t *Tracker) askUnheard() error {
return nil
}
// unheardOf is the pull requests an issue names that the tracker keeps no record for.
func unheardOf(is *Issue) []string {
var out []string
for _, pr := range fixPullRequests(is) {
if fixOf(is, pr) == nil {
out = append(out, pr)
}
}
return out
}
// openPullRequest says whether an issue names a pull request not merged yet, or one whose delivery the tracker
// never heard of: work in review is work, and a fix unheard of is not taken for none. One closed without merging
// is not open.
+30 -33
View File
@@ -454,9 +454,22 @@ func (t *Tracker) OnDelivered(e stdio.Envelope) error {
at = t.now()
}
var changed []uint64
learned := 0
for n, is := range all {
for _, pr := range liveFixes(is) {
f := fixOf(is, pr)
if f == nil && learned < MostSweepAsks {
// A pull request named and kept no record for (merged before the tracker heard deliveries): it learns its
// merge now, so this very event can cover it (issue 1950).
learned++
if ok, err := t.learnFix(n, pr); err != nil {
t.Logf("[mesh-issues] the merge of %s (issue %d) could not be learned (asked again at the next event): %v", pr, n, err)
} else if ok {
if fresh, _, err := read(t.Store, n); err == nil {
is, f = fresh, fixOf(fresh, pr)
}
}
}
if f != nil && f.Merged && !contains(is.Delivered, pr) && f.MergedAs == "" {
t.fillMerge(n, f, pr)
}
@@ -624,6 +637,10 @@ type Delivery struct {
Repository string
Group string
MergedAs string
// Pulled says the answer carried the forge's pull request (merged, closed, the commit its merge landed as); Closed that it
// was closed without merging.
Pulled bool
Closed bool
}
// heard is what a delivery answered, as a fix keeps it.
@@ -662,16 +679,18 @@ func busDeliveryState(owner string) DeliveryState {
Group string `json:"group"`
MergedAs string `json:"merged_as"`
} `json:"delivery"`
Pull struct {
Merged bool `json:"merged"`
} `json:"pull"`
Pull *pullFacts `json:"pull"`
}
if err := json.Unmarshal(raw, &out); err != nil {
return Delivery{}, fmt.Errorf("mesh-delivery's checks answered no delivery: %.200s", raw)
}
return Delivery{State: out.Delivery.State, Why: out.Delivery.Waits, Since: out.Delivery.Since.UTC(), Merged: out.Pull.Merged,
d := Delivery{State: out.Delivery.State, Why: out.Delivery.Waits, Since: out.Delivery.Since.UTC(),
Class: out.Delivery.Class, Opened: nonZero(out.Delivery.Opened), MergedAt: nonZero(out.Delivery.MergedAt),
Repository: strings.ToLower(owner + "/" + repository), Group: out.Delivery.Group, MergedAs: out.Delivery.MergedAs}, nil
Repository: strings.ToLower(owner + "/" + repository), Group: out.Delivery.Group, MergedAs: out.Delivery.MergedAs}
if out.Pull != nil {
d.fromPull(*out.Pull, owner+"/"+repository)
}
return d, nil
}
}
@@ -695,34 +714,12 @@ func (t *Tracker) OnMoved(e stdio.Envelope) error {
if t.Deliveries != nil {
// Each ask up to the bus's 30 seconds, under the runtime's two minutes; one that fails, or more left to
// ask, has the event offered again, so a delivery never asked ends in max-deliveries, not in silence.
asked := 0
for _, pr := range fixPullRequests(is) {
if fixOf(is, pr) != nil {
continue
}
if asked == MostDeliveryAsks {
return fmt.Errorf("issue %d names more pull requests than are asked at once; the rest at the next offer", body.Number)
}
asked++
repo, number, _ := strings.Cut(pr, "#")
d, err := t.Deliveries(repo, number)
if err != nil {
return fmt.Errorf("the delivery of %s (issue %d) could not be asked: %v", pr, body.Number, err)
}
state := d.State
if state == "" {
// The delivery knows nothing of it yet: not kept, so it is asked again at the next move, and its own
// events say it when they come.
continue
}
// The delivery's own time, so a transition stamped the same moment is not taken as older.
since := d.Since
if since.IsZero() {
since = t.now()
}
if _, err := t.recordFix(pr, d.heard(since), body.Number); err != nil {
return err
}
asked, _, err := t.learnUnheard(body.Number, is, MostDeliveryAsks)
if err != nil {
return err
}
if asked == MostDeliveryAsks && len(unheardOf(is)) > asked {
return fmt.Errorf("issue %d names more pull requests than are asked at once; the rest at the next offer", body.Number)
}
}
return t.afterFix(body.Number)
@@ -0,0 +1,181 @@
package main
// A fix whose pull request the tracker keeps no record for (merged before the tracker heard the delivery, or linked after it
// moved) learns its merge here, so the view's `delivered` event can cover it like any recorded fix (issue 1950). The delivery
// is asked first (its `checks` carries the forge's own pull request beside its delivery); the forge is asked when the delivery
// cannot say. One that neither can say is left for the next time and logged; a pull request closed without merging becomes
// the withdrawn state the tracker already knows.
import (
"encoding/json"
"errors"
"fmt"
"strings"
"time"
stdio "git.novox.be/novox/mesh-sdk/go"
)
// pullFacts reads the forge's pull request block, as gitea_get_pull_request and mesh-delivery's checks both carry it.
type pullFacts struct {
Merged bool `json:"merged"`
State string `json:"state"`
MergeCommitSHA string `json:"merge_commit_sha"`
MergedAt *time.Time `json:"merged_at"`
}
// fromPull completes a delivery with what the forge's pull request says: whether it is merged or closed, the commit its
// merge landed as and when.
func (d *Delivery) fromPull(p pullFacts, repository string) {
d.Pulled = true
d.Merged = p.Merged
d.Closed = p.State == "closed" && !p.Merged
if d.Repository == "" {
d.Repository = strings.ToLower(repository)
}
if p.Merged {
if d.MergedAs == "" {
d.MergedAs = p.MergeCommitSHA
}
if d.MergedAt == nil {
d.MergedAt = nonZero(p.MergedAt)
}
}
}
// busPull asks the forge for one pull request (gitea_get_pull_request).
func busPull(owner string) DeliveryState {
return func(repository, number string) (Delivery, error) {
var n int
if _, err := fmt.Sscanf(number, "%d", &n); err != nil || n <= 0 {
return Delivery{}, fmt.Errorf("%q is not a pull request number", number)
}
raw, err := stdio.Ask("gitea.gitea_get_pull_request", map[string]any{"owner": owner, "repo": repository, "number": n})
if err != nil {
return Delivery{}, err
}
var out struct {
Pull *pullFacts `json:"pull"`
Answer struct {
Pull *pullFacts `json:"pull"`
} `json:"answer"`
}
if json.Unmarshal(raw, &out) != nil {
return Delivery{}, errors.New("the forge's gitea_get_pull_request is not readable")
}
p := out.Pull
if p == nil {
p = out.Answer.Pull
}
if p == nil {
return Delivery{}, errors.New("the forge's gitea_get_pull_request answered no pull request")
}
var d Delivery
d.fromPull(*p, owner+"/"+repository)
return d, nil
}
}
// learnFix gives a pull request an issue names and keeps no Fix record for its record: from the delivery's own state when it has
// one, else from the pull request itself (the delivery's, then the forge's). It answers whether a record was kept. A pull request
// still open is not kept (its own events say it when they come); one neither can say anything of is left for next time.
func (t *Tracker) learnFix(n uint64, pr string) (bool, error) {
repo, number, _ := strings.Cut(pr, "#")
var d Delivery
var derr error
if t.Deliveries != nil {
d, derr = t.Deliveries(repo, number)
}
if derr == nil && d.State != "" {
since := d.Since
if since.IsZero() {
since = t.now()
}
ch, err := t.recordFix(pr, d.heard(since), n)
return len(ch) > 0, err
}
if derr != nil || !d.Pulled {
if t.Pull == nil {
return false, derr
}
f, ferr := t.Pull(repo, number)
if ferr != nil {
if derr != nil {
return false, derr
}
t.Logf("[mesh-issues] the pull request %s (issue %d) could not be read from the forge (asked again later): %v", pr, n, ferr)
return false, nil
}
d = f
}
var h heardFix
switch {
case d.Merged:
at := t.now()
if d.MergedAt != nil {
at = *d.MergedAt
}
h = heardFix{State: "merged", Merged: true, At: at, MergedAt: d.MergedAt, Repository: d.Repository, MergedAs: d.MergedAs}
case d.Closed:
h = heardFix{State: "closed", Why: "closed without merging", At: t.now()}
default:
return false, nil
}
ch, err := t.recordFix(pr, h, n)
return len(ch) > 0, err
}
// learnUnheard learns, for at most most pull requests, each fix of issue n with no record; it answers whether any was kept.
func (t *Tracker) learnUnheard(n uint64, is *Issue, most int) (int, bool, error) {
asked, kept := 0, false
for _, pr := range fixPullRequests(is) {
if fixOf(is, pr) != nil {
continue
}
if asked == most {
break
}
asked++
ok, err := t.learnFix(n, pr)
if err != nil {
return asked, kept, fmt.Errorf("the delivery of %s (issue %d) could not be asked: %v", pr, n, err)
}
kept = kept || ok
}
return asked, kept, nil
}
// LearnFixes is the tool that asks again for one issue: every pull request its fixed-by names with no record is asked of the
// delivery and the forge now. It answers what each is kept as.
func (t *Tracker) LearnFixes(n uint64) (map[string]any, error) {
is, _, err := read(t.Store, n)
if err != nil {
return nil, err
}
if _, _, err := t.learnUnheard(n, is, MostSweepAsks*2); err != nil {
return nil, err
}
if err := t.afterFix(n); err != nil {
return nil, err
}
is, _, err = read(t.Store, n)
if err != nil {
return nil, err
}
out := map[string]string{}
for _, pr := range fixPullRequests(is) {
switch f := fixOf(is, pr); {
case f == nil:
out[pr] = "nothing heard: neither the delivery nor the forge said"
case withdrawn(f):
out[pr] = "withdrawn: closed without merging"
case contains(is.Delivered, pr):
out[pr] = "delivered"
case f.Merged:
out[pr] = "merged as " + shortCommit(f.MergedAs) + ", not delivered"
default:
out[pr] = f.State
}
}
return map[string]any{"number": n, "fixes": out, "delivery": DeliveryLine(is)}, nil
}
@@ -0,0 +1,126 @@
package main
import (
"encoding/json"
"errors"
"strings"
"testing"
"time"
)
// The forge's answer for novox/mesh-delivery#5, read from gitea.gitea_get_pull_request on 2026-10-27 (shortened): merged, closed,
// its merge commit and moment. mesh-delivery's checks carries the same `pull` block and no `delivery` block for it.
const forgePullMerged = `{"pull":{"base":"main","head":"feat/delivery-parts","merged":true,"merged_at":"2026-10-19T18:05:16Z",` +
`"merge_commit_sha":"1325c8b452c8fa65c478af6815887870a1f42e7b","number":5,"state":"closed"}}`
func TestAPullRequestWithNoFixRecordLearnsItsMerge(t *testing.T) {
mergedAt := time.Date(2026, 10, 19, 18, 5, 16, 0, time.UTC)
merged := Delivery{Pulled: true, Merged: true, MergedAs: "1325c8b452c8fa65c478af6815887870a1f42e7b", MergedAt: &mergedAt, Repository: "novox/mesh-catalog"}
closed := Delivery{Pulled: true, Closed: true}
open := Delivery{Pulled: true}
for _, c := range []struct {
name string
delivery Delivery
deliveryEr error
forge *Delivery
forgeEr error
want string // the delivery line after learning
wantErr bool
}{
{name: "mesh-delivery's pull says merged", delivery: merged, want: "fix merged, not delivered: mesh-catalog#50"},
{name: "mesh-delivery cannot, the forge says merged", deliveryEr: errors.New("down"), forge: &merged,
want: "fix merged, not delivered: mesh-catalog#50"},
{name: "mesh-delivery says nothing of the pull, the forge says merged", forge: &merged, want: "fix merged, not delivered: mesh-catalog#50"},
{name: "closed without merging is withdrawn", delivery: closed, want: "fix withdrawn: mesh-catalog#50 closed without merging"},
{name: "still open stays unheard", delivery: open, want: "nothing heard from the delivery of mesh-catalog#50"},
{name: "a forge that cannot say leaves it", forgeEr: errors.New("forge down"), want: "nothing heard from the delivery of mesh-catalog#50"},
{name: "neither can say", deliveryEr: errors.New("down"), forgeEr: errors.New("forge down"), want: "nothing heard from the delivery of mesh-catalog#50", wantErr: true},
} {
t.Run(c.name, func(t *testing.T) {
h := newHarness(t)
n := fixIssue(t, h, "mesh-catalog#50")
h.Deliveries = func(string, string) (Delivery, error) { return c.delivery, c.deliveryEr }
h.Pull = func(string, string) (Delivery, error) {
if c.forge != nil {
return *c.forge, nil
}
return Delivery{}, errOr(c.forgeEr)
}
_, err := h.LearnFixes(n)
if (err != nil) != c.wantErr {
t.Fatalf("error %v, wanted error %v", err, c.wantErr)
}
if v, _ := h.View(n); v.Delivery != c.want {
t.Fatalf("%q, wanted %q", v.Delivery, c.want)
}
})
}
}
func errOr(e error) error {
if e == nil {
return errors.New("the forge has no such pull request")
}
return e
}
// A fix merged before the tracker heard deliveries: the delivered event itself learns its merge and covers it, a resolved issue's
// included; and the sweep learns it with no event.
func TestAFixMergedBeforeTheTrackerHeardDeliveriesIsDeliveredByTheNextEventOfItsRepository(t *testing.T) {
mergedAs := "1325c8b452c8fa65c478af6815887870a1f42e7b"
forge := func(string, string) (Delivery, error) {
return Delivery{Pulled: true, Merged: true, MergedAs: mergedAs, Repository: "novox/mesh-catalog"}, nil
}
contains := func(_, _, commit, in string) (bool, error) {
return commit == mergedAs && in == "ffffffffffff0000", nil
}
heads := map[string]string{"novox/mesh-catalog": "ffffffffffff0000"}
delivered := "delivered to every machine it should reach"
h := newHarness(t)
n := fixIssue(t, h, "mesh-catalog#51")
h.Pull, h.Contains = forge, contains
viewSays(t, h, 30, "", heads)
if v, _ := h.View(n); v.Delivery != delivered {
t.Fatalf("the event did not cover the old fix: %q", v.Delivery)
}
if f := fixOf(get(t, h, n), "mesh-catalog#51"); f == nil || f.MergedAs != mergedAs || f.Repository != "novox/mesh-catalog" || f.MergedAt != nil && f.MergedAt.IsZero() {
t.Fatalf("fix %+v", f)
}
// A resolved issue is learned by the sweep, which then waits for the next event, not for ever.
h2 := newHarness(t)
m := fixIssue(t, h2, "mesh-catalog#52")
if _, err := h2.Move(MoveAsk{Number: m, Status: StatusResolved, FixedBy: []string{"mesh-catalog#52"}}); err != nil {
t.Fatal(err)
}
if v, _ := h2.View(m); !strings.HasPrefix(v.Delivery, "nothing heard") {
t.Fatalf("before: %q", v.Delivery)
}
h2.Pull, h2.Contains = forge, contains
if _, err := h2.Sweep(); err != nil {
t.Fatal(err)
}
if v, _ := h2.View(m); v.Delivery != "fix merged, not delivered: mesh-catalog#52" {
t.Fatalf("after the sweep: %q", v.Delivery)
}
viewSays(t, h2, 31, "", heads)
if v, _ := h2.View(m); v.Delivery != delivered {
t.Fatalf("after the event: %q", v.Delivery)
}
}
func TestTheForgesRealPullAnswerIsReadAsMerged(t *testing.T) {
var out struct {
Pull pullFacts `json:"pull"`
}
if err := json.Unmarshal([]byte(forgePullMerged), &out); err != nil {
t.Fatal(err)
}
var d Delivery
d.fromPull(out.Pull, "novox/mesh-delivery")
if !d.Pulled || !d.Merged || d.Closed || d.MergedAs != "1325c8b452c8fa65c478af6815887870a1f42e7b" ||
d.MergedAt == nil || d.MergedAt.Format(time.RFC3339) != "2026-10-19T18:05:16Z" || d.Repository != "novox/mesh-delivery" {
t.Fatalf("%+v", d)
}
}
+22 -1
View File
@@ -23,7 +23,7 @@ const Seat = "issue-tracker"
// Verbs are the seat's verbs, as the manifest declares and claims them.
var Verbs = []string{"open", "get", "list", "move", "note", "link", "milestones", "milestone-set", "milestone-move", "import",
"groom", "set", "tracking", "reserve", "fix-times"}
"groom", "set", "tracking", "reserve", "fix-times", "learn-fixes"}
func logf(format string, a ...any) { fmt.Fprintf(os.Stderr, format+"\n", a...) }
@@ -105,6 +105,9 @@ func main() {
t.Deliveries = func(repository, number string) (Delivery, error) {
return busDeliveryState(forgeOwner())(repository, number)
}
t.Pull = func(repository, number string) (Delivery, error) {
return busPull(forgeOwner())(repository, number)
}
t.Approval = busApproval(forgeOwner)
t.Contains = busContains
if len(os.Args) > 1 && os.Args[1] == "dry-run" {
@@ -921,5 +924,23 @@ func Tools(t *Tracker) []stdio.Tool {
}
return t.FixTimes(int(days))
}},
{Name: Seat + ".learn-fixes",
Description: "Ask again for the fixes of one issue the tracker keeps no record of: each pull request its fixed-by names that " +
"the delivery was never heard of (merged before the tracker heard deliveries, or linked after) is asked of mesh-delivery " +
"and, when it cannot say, of the forge, and kept as merged (with the commit its merge landed as) or as withdrawn when it " +
"was closed without merging; a merge is then matched to the next delivered revision of its repository. Answers what each " +
"pull request is kept as and the issue's delivery line. A pull request still open, or one neither can say, stays unheard.",
Input: schema([]string{"number"}, map[string]any{"number": number("the issue's number")}),
Run: func(m map[string]any) (any, error) {
a, err := readArgs("learn-fixes", m, "number")
if err != nil {
return nil, err
}
n, err := a.number("number", true)
if err != nil {
return nil, err
}
return t.LearnFixes(n)
}},
}
}
@@ -37,6 +37,8 @@ type Tracker struct {
Judge Judgement
// Deliveries answers a pull request's delivery (ADR 0279 §3, ADR 0281); nil asks nothing.
Deliveries DeliveryState
// Pull reads one pull request from the forge (gitea_get_pull_request) for a fix the delivery cannot say anything of; nil asks nothing.
Pull DeliveryState
// Contains asks the forge whether a commit is contained in another (gitea_contains); nil asks nothing, and a delivered
// revision then marks only the fixes it names (OnDelivered).
Contains func(owner, repo, commit, in string) (bool, error)