novox/mesh-catalog
18
0
Fork 0
Code Issues Pull Requests 3 Actions Packages Projects Releases Wiki Activity

mesh-issues: review — delivered stays delivered against out-of-order transitions, a stuck warning is cleared and retried, the ready-to-verify note comes before its claim, a fix no longer named is not swept, an empty delivery is asked again (hq ADR 0281)

Browse Source
This commit is contained in:
jochen
2026-10-10 18:11:45 +02:00
parent c4f84b9d4d
commit 638076f939
7 changed files with 150 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files
modules/mesh-issues/README.md
+1 -1
View File
@@ -97,7 +97,7 @@ nothing.
Every five minutes the module looks for a fix merged and not delivered after `stuck-after` (default `2h`): it is
said once — a note on the issue and a warning on the operator's channel (`seat:operator-channel.notify`, key
`issue-tracker.fix-stuck.<number>`), cleared there when every fix of the issue is delivered. A module that runs only
its bundle cannot raise a controller condition (hq ADR 0256), so the warning is the channel's, not the controller's.
its bundle cannot raise a controller condition (hq ADR 0240 rule 8, ADR 0256), so the warning is the channel's, not the controller's.
## The import of hq's issues (slice 3)
modules/mesh-issues/cmd/mesh-issues/delivery.go
+77 -30
View File
@@ -1,6 +1,6 @@
package main
// Whether an issue's fix is delivered (novox/hq ADR 0280). The tracker hears mesh-delivery's `transition` event
// Whether an issue's fix is delivered (novox/hq ADR 0281). The tracker hears mesh-delivery's `transition` event
// for every pull request an issue's fixed-by names and keeps, per pull request, the state the delivery last said
// — merged or not, held and why, covered by a later merge, delivered. From that it derives one line for `get` and
// `list`: a measured fact, never set by a verb. Delivered is not resolved: when every fix of an issue not yet
@@ -131,7 +131,14 @@ func (t *Tracker) recordFix(pr, state, why string, merged bool, at time.Time, on
if (only != 0 && n != only) || !contains(pullRequests(is), pr) {
continue
}
if f := fixOf(is, pr); f != nil && f.State == state && f.Why == why && f.Merged == (merged || f.Merged) {
kept := cut(why, MostReasonKept)
if f := fixOf(is, pr); f != nil && (at.Before(f.Since) || (f.State == state && f.Why == kept && f.Merged == (merged || f.Merged))) {
// Older than what is kept, or nothing new: transitions can arrive out of order.
continue
}
if contains(is.Delivered, pr) && !reached(state) {
// Delivered stays delivered: a later state that has not reached the machines is a newer delivery of
// the same pull request at most, and an older one is history.
continue
}
if _, err := t.change(n, func(i *Issue) error {
@@ -140,7 +147,7 @@ func (t *Tracker) recordFix(pr, state, why string, merged bool, at time.Time, on
i.Fixes = append(i.Fixes, Fix{PullRequest: pr})
f = &i.Fixes[len(i.Fixes)-1]
}
f.State, f.Why, f.Since = state, cut(why, MostReasonKept), at
f.State, f.Why, f.Since = state, kept, at
if merged && !f.Merged {
f.Merged = true
m := at
@@ -167,43 +174,72 @@ func (t *Tracker) afterFix(n uint64) error {
if err != nil {
return err
}
if err := t.clearStuck(is); err != nil {
t.Logf("[mesh-issues] the stuck fix of %d could not be cleared on the operator's channel (tried again at the next sweep): %v", n, err)
}
if allDelivered(is) {
for _, f := range is.Fixes {
if f.StuckSaid && t.Notify != nil {
if err := t.Notify(stuckKey(n), "", "", "", "", "the fix of issue "+keyOf(n)+" is delivered", true); err != nil {
t.Logf("[mesh-issues] the stuck fix of %d could not be cleared on the operator's channel: %v", n, err)
}
break
}
}
if unresolved(is) {
prs := pullRequests(is)
sort.Strings(prs)
claimed := false
if _, err := t.change(n, func(i *Issue) error {
claimed = false
if strings.Join(i.VerifyFor, ",") == strings.Join(prs, ",") {
return nil
if strings.Join(is.VerifyFor, ",") != strings.Join(prs, ",") {
// The note first, kept once by its words; then the claim; then the event. A failure before the claim
// is tried again at the next offer, and the note is not written twice.
prefix := fmt.Sprintf("Ready to verify: every fix of this issue is delivered (%s).", strings.Join(prs, ", "))
if !noted(is, prefix) {
if _, err := t.note(GroomBy, n, prefix+" Delivered is not resolved: prove the acceptance live, then "+
"resolve it. (novox/hq ADR 0281)"); err != nil {
return err
}
}
i.VerifyFor, claimed = prs, true
return nil
}); err != nil {
return err
}
if claimed {
// At most once, as `ready`: a note that fails after the claim is not said again.
if _, err := t.note(GroomBy, n, fmt.Sprintf("Ready to verify: every fix of this issue is delivered (%s). "+
"Delivered is not resolved: prove the acceptance live, then resolve it. (novox/hq ADR 0280)",
strings.Join(prs, ", "))); err != nil {
claimed := false
if _, err := t.change(n, func(i *Issue) error {
claimed = false
if strings.Join(i.VerifyFor, ",") == strings.Join(prs, ",") {
return nil
}
i.VerifyFor, claimed = prs, true
return nil
}); err != nil {
return err
}
t.emit(EventReadyToVerify, map[string]any{"number": n, "fixed-by": prs, "at": t.now()})
if claimed {
t.emit(EventReadyToVerify, map[string]any{"number": n, "fixed-by": prs, "at": t.now()})
}
}
}
}
return t.letGo(n)
}
// stillStuck says whether an issue has a fix said stuck that is still named, not delivered, and the issue not
// closed as wontfix.
func stillStuck(is *Issue) bool {
if is.Status == StatusWontfix {
return false
}
named := pullRequests(is)
for _, f := range is.Fixes {
if f.StuckSaid && contains(named, f.PullRequest) && !contains(is.Delivered, f.PullRequest) {
return true
}
}
return false
}
// clearStuck clears the warning said for an issue on the operator's channel once nothing it was said for is
// stuck — delivered, no longer named, or the issue closed as wontfix; one that fails is tried again at the next
// sweep.
func (t *Tracker) clearStuck(is *Issue) error {
if !is.StuckOpen || stillStuck(is) || t.Notify == nil {
return nil
}
if err := t.Notify(stuckKey(is.Number), "", "", "", "", "the fix of issue "+keyOf(is.Number)+" is delivered or no longer waited for", true); err != nil {
return err
}
_, err := t.change(is.Number, func(i *Issue) error { i.StuckOpen = false; return nil })
return err
}
func stuckKey(n uint64) string { return "issue-tracker.fix-stuck." + keyOf(n) }
func prKey(repository string, number int) string {
@@ -310,7 +346,9 @@ func (t *Tracker) OnMoved(e stdio.Envelope) error {
}
state := d.State
if state == "" {
state = "unknown"
// 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
}
if _, err := t.recordFix(pr, state, d.Why, afterMerge(state), t.now(), body.Number); err != nil {
return err
@@ -359,10 +397,18 @@ func (t *Tracker) Sweep() (int, error) {
said := 0
for n, is := range all {
if is.Status == StatusWontfix {
if err := t.clearStuck(is); err != nil {
t.Logf("[mesh-issues] the stuck fix of %d could not be cleared on the operator's channel: %v", n, err)
}
continue
}
if err := t.clearStuck(is); err != nil {
t.Logf("[mesh-issues] the stuck fix of %d could not be cleared on the operator's channel: %v", n, err)
}
named := pullRequests(is)
for _, f := range is.Fixes {
if !f.Merged || f.MergedAt == nil || reached(f.State) || f.StuckSaid || now.Sub(*f.MergedAt) < bound {
if !contains(named, f.PullRequest) || contains(is.Delivered, f.PullRequest) || !f.Merged || f.MergedAt == nil ||
reached(f.State) || f.StuckSaid || now.Sub(*f.MergedAt) < bound {
continue
}
state := f.State
@@ -372,7 +418,7 @@ func (t *Tracker) Sweep() (int, error) {
prefix := fmt.Sprintf("Stuck: the fix %s", f.PullRequest)
if !noted(is, prefix) {
if _, err := t.note(GroomBy, n, fmt.Sprintf("%s was merged %s ago and is not delivered (%s). A merged fix "+
"that does not reach the machines is said on the operator's channel. (novox/hq ADR 0280)", prefix,
"that does not reach the machines is said on the operator's channel. (novox/hq ADR 0281)", prefix,
now.Sub(*f.MergedAt).Round(time.Minute), state)); err != nil {
return said, err
}
@@ -394,6 +440,7 @@ func (t *Tracker) Sweep() (int, error) {
if g := fixOf(i, pr); g != nil {
g.StuckSaid = true
}
i.StuckOpen = t.Notify != nil
return nil
}); err != nil {
return said, err
modules/mesh-issues/cmd/mesh-issues/delivery_test.go
+56
View File
@@ -188,3 +188,59 @@ func TestAFixIsNamedWhileTheIssueStaysLocated(t *testing.T) {
t.Fatalf("%+v", is)
}
}
// Transitions out of order never take a delivered fix back, and a delivered fix is never said stuck; a clear
// that fails is tried again; a fix no longer named is not swept; an empty answer from the delivery is asked again.
func TestDeliveredStaysDeliveredAndClearsAreRetried(t *testing.T) {
h := newHarness(t)
n := h.open(t, "x")
resolve(t, h, n, "mesh-catalog#9")
note := &notified{}
h.Notify = note.notify
merged := time.Date(2026, 10, 10, 9, 0, 0, 0, time.UTC)
transition(t, h, 9, "held", "waits", "abc", merged)
if said, _ := h.Sweep(); said != 1 {
t.Fatal("not said stuck")
}
note.err = errStuck
transition(t, h, 9, "delivered", "", "abc", merged.Add(time.Hour))
transition(t, h, 9, "held", "an older word", "abc", merged.Add(30*time.Minute)) // out of order
if v, _ := h.View(n); v.Delivery != "delivered to every machine it should reach" {
t.Fatal(v.Delivery)
}
if len(note.clear) != 0 || !get(t, h, n).StuckOpen {
t.Fatal("cleared while the channel failed")
}
note.err = nil
if said, _ := h.Sweep(); said != 0 || len(note.clear) != 1 || get(t, h, n).StuckOpen {
t.Fatalf("said %d, cleared %q", said, note.clear)
}
// A fix no longer named is not swept, and its warning is cleared.
h2 := newHarness(t)
m := h2.open(t, "y")
if _, err := h2.Move(MoveAsk{Number: m, Status: StatusLocated, LocatedIn: []string{"mesh-catalog modules/records"}, FixedBy: []string{"mesh-catalog#9"}}); err != nil {
t.Fatal(err)
}
n2 := &notified{}
h2.Notify = n2.notify
transition(t, h2, 9, "held", "waits", "abc", merged)
_, _ = h2.Sweep()
if _, err := h2.Move(MoveAsk{Number: m, Status: StatusLocated, FixedBy: []string{"mesh-catalog#10"}}); err != nil {
t.Fatal(err)
}
if said, _ := h2.Sweep(); said != 0 || len(n2.said) != 1 || len(n2.clear) != 1 {
t.Fatalf("said %d %q, cleared %q", said, n2.said, n2.clear)
}
if _, err := h2.Move(MoveAsk{Number: m, Status: StatusLocated, FixedBy: []string{"mesh-catalog#10"}}); err == nil {
t.Fatal("a move that changes nothing was taken")
}
// The delivery knows nothing yet: not kept, asked again at the next move.
asks := 0
h2.Deliveries = func(string, string) (Delivery, error) { asks++; return Delivery{}, nil }
for i := 0; i < 2; i++ {
_ = h2.OnMoved(stdio.Envelope{Body: json.RawMessage(`{"number":365}`)})
}
if v, _ := h2.View(m); asks != 2 || v.Delivery != "nothing heard from the delivery of mesh-catalog#10" {
t.Fatalf("%d asks, %q", asks, v.Delivery)
}
}
modules/mesh-issues/cmd/mesh-issues/issue.go
+3 -1
View File
@@ -91,11 +91,13 @@ type Issue struct {
SuggestedOwner string `json:"suggested-owner,omitempty"`
// Delivered is the pull requests of FixedBy the delivery said are delivered.
Delivered []string `json:"delivered,omitempty"`
// Fixes is each pull request of FixedBy as the delivery last said it (ADR 0280): measured, never set by a
// Fixes is each pull request of FixedBy as the delivery last said it (ADR 0281): measured, never set by a
// verb.
Fixes []Fix `json:"fixes,omitempty"`
// VerifyFor is the pull requests it was last said ready to verify for, so it is said once.
VerifyFor []string `json:"verify-for,omitempty"`
// StuckOpen is the warning on the operator's channel said for a stuck fix and not yet cleared there.
StuckOpen bool `json:"stuck-open,omitempty"`
// ReadyFor is the issues it depended on when it was last said ready (ADR 0279 §3), so one spell of
// readiness is said once however many dependencies let it go at one delivery.
ReadyFor []uint64 `json:"ready-for,omitempty"`
modules/mesh-issues/cmd/mesh-issues/main.go
+2 -2
View File
@@ -41,7 +41,7 @@ type Settings struct {
// ForgeOwner is the owner on the forge of the repositories fixed-by names, for asking a pull request's
// delivery (ADR 0279 §3).
ForgeOwner string `json:"forge-owner,omitempty"`
// StuckAfter is how long a fix may be merged and not delivered before it is said stuck (ADR 0280), as a
// StuckAfter is how long a fix may be merged and not delivered before it is said stuck (ADR 0281), as a
// duration such as 2h; DefaultStuckAfter when empty.
StuckAfter string `json:"stuck-after,omitempty"`
}
@@ -122,7 +122,7 @@ func hearOpened(t *Tracker) {
}
}
// sweep says, every five minutes, every fix merged and not delivered past the bound (ADR 0280).
// sweep says, every five minutes, every fix merged and not delivered past the bound (ADR 0281).
func sweep(t *Tracker) {
for range time.Tick(5 * time.Minute) {
if n, err := t.Sweep(); err != nil {
modules/mesh-issues/cmd/mesh-issues/ready.go
+1 -1
View File
@@ -268,7 +268,7 @@ type View struct {
NeededBy []uint64 `json:"needed-by,omitempty"`
WaitsFor []uint64 `json:"waits-for,omitempty"`
Ready bool `json:"ready"`
// Delivery is whether its fix is delivered, in words (ADR 0280).
// Delivery is whether its fix is delivered, in words (ADR 0281).
Delivery string `json:"delivery,omitempty"`
}
modules/mesh-issues/cmd/mesh-issues/tracker.go
+10 -5
View File
@@ -30,9 +30,9 @@ type Tracker struct {
Records Records
// Judge is the triage's way to a model (ADR 0278, ADR 0279); nil judges nothing.
Judge Judgement
// Deliveries answers a pull request's delivery (ADR 0279 §3, ADR 0280); nil asks nothing.
// Deliveries answers a pull request's delivery (ADR 0279 §3, ADR 0281); nil asks nothing.
Deliveries DeliveryState
// Notify tells the operator's channel of a stuck fix (ADR 0280); nil says nothing there.
// Notify tells the operator's channel of a stuck fix (ADR 0281); nil says nothing there.
Notify Notifier
groom grooming
}
@@ -397,9 +397,14 @@ func (t *Tracker) move(by string, a MoveAsk) (*Moved, error) {
is, err := t.change(a.Number, func(is *Issue) error {
changed = nil
from = is.Status
// A field counts as changed only when its value does, so a move that changes nothing is refused.
set := func(name string, cond bool, apply func()) {
if cond {
apply()
if !cond {
return
}
before, _ := json.Marshal(is)
apply()
if after, _ := json.Marshal(is); string(before) != string(after) {
changed = append(changed, name)
}
}
@@ -626,7 +631,7 @@ type Row struct {
Ready bool `json:"ready"`
// SuggestedOwner is the triage's or a person's suggestion, for an issue located nowhere yet.
SuggestedOwner string `json:"suggested-owner,omitempty"`
// Delivery is whether its fix is delivered, in words (ADR 0280); empty when fixed-by names no pull request.
// Delivery is whether its fix is delivered, in words (ADR 0281); empty when fixed-by names no pull request.
Delivery string `json:"delivery,omitempty"`
}