diff --git a/modules/mesh-delivery/cmd/mesh-delivery/issue611_test.go b/modules/mesh-delivery/cmd/mesh-delivery/issue611_test.go new file mode 100644 index 00000000..dd12f090 --- /dev/null +++ b/modules/mesh-delivery/cmd/mesh-delivery/issue611_test.go @@ -0,0 +1,150 @@ +package main + +import ( + "sort" + "strings" + "testing" + "time" +) + +// novox/hq issue 611: mesh-controller #293 changed one test file. Its check's plan, made while another walk built +// build-agent and route-proxy, read them whole and named them; at the cut their recorded build sources held none of +// its files, so its walk was done with no tier, "moved nothing the mesh holds: nothing to walk". The stranded rule +// (issue 474) read that as "ended without building build-agent, route-proxy", stopped it, and the operator was asked +// about a change already on every machine. The controller now says per module that the build matched what runs +// (the walk's delivery.unchanged), and such a delivery is delivered: its merge moves nothing (ADR 0270 §1). A walk +// that says neither — not built, not unchanged — still stops it. + +const ( + head293 = "94c928b4c7e1d2aa" + merge293 = "8f1aec0f2b9d6e11" + walk293 = "plan-1791867648768608552" +) + +var at293 = time.Date(2026, 10, 13, 5, 10, 0, 0, time.UTC) + +// matched is the why the controller gives a module it says unchanged. +const matched = "its build source in novox/mesh-controller holds none of the 1 changed file(s) " + + "(e.g. internal/builder/replay363_test.go): the build the mesh holds is its build" + +// replay293 is #293 checked moving build-agent and route-proxy, merged, and its walk done with no tier, saying the +// modules named unchanged. +func replay293(t *testing.T, unchanged map[string]string) *world { + t.Helper() + w := newWorldOver(t, newMemStore(), newFakeController(), newFakeForge(), at293.Add(-time.Minute)) + p := pr(ctl474, 293, head293, "fix/363-replay", "internal/builder/replay363_test.go") + w.h.PullUpdated(p) + w.h.Checked(verdictMoving(ctl474, 293, head293, "build-agent", "route-proxy")) + w.later(time.Minute) + w.h.PullMerged(mergedAt(p, merge293, at293)) + w.later(90 * time.Second) + note := nothingNote + if len(unchanged) > 0 { + note += "; unchanged, the build the mesh holds is theirs: " + strings.Join(sortedKeys(unchanged), ", ") + } + w.h.WalkMoved(w.ctl.put(Walk{ID: walk293, Repository: ctl474, Branch: "main", Commit: merge293, Created: w.now, + Updated: w.now, State: walkDone, Tiers: [][]string{}, Modules: map[string]*WalkModule{}, Note: note, + Delivery: &WalkDelivery{Awaits: "mesh-delivery", Merges: []WalkMerge{{Repository: ctl474, Commit: merge293}}, + Unchanged: unchanged}})) + w.settleAll() + return w +} + +// The live case: every module its plan named, the walk said matched what runs. Delivered at once, naming them, a +// success on the pull request, and nothing listed by stalled or asked of the operator, then or past the grace. +func TestAMergeWhoseModulesMatchWhatRunsIsDelivered(t *testing.T) { + w := replay293(t, map[string]string{"build-agent": matched, "route-proxy": matched}) + id := IDOf(ctl474, head293) + d := w.delivery(id) + if d.State != Delivered { + t.Fatalf("#293, its modules said unchanged by its walk, is %s, waiting for %q", d.State, w.h.waitsFor(d)) + } + why := d.Transitions[len(d.Transitions)-1].Why + for _, want := range []string{walk293, "build-agent, route-proxy", "matched what runs"} { + if !strings.Contains(why, want) { + t.Fatalf("delivered as %q, not saying %q", why, want) + } + } + if d.Stranded != "" { + t.Fatalf("it was read as stranded: %q", d.Stranded) + } + status := w.forge.statuses["novox/mesh-controller@"+head293+" mesh/delivery"] + if !strings.HasPrefix(status, "success ") { + t.Fatalf("mesh/delivery says %q", status) + } + w.later(strandedAfter + time.Minute) + w.settleAll() + if s := w.state(id); s != Delivered { + t.Fatalf("past the grace it is %s", s) + } + if lines := w.h.Stalled(); len(lines) != 0 { + t.Fatalf("stalled lists %+v", lines) + } +} + +// A walk that said only one of its modules unchanged left the other unbuilt: it is still stopped past the grace, +// naming only the module nothing delivered. +func TestAMergeWithAModuleNeitherBuiltNorUnchangedIsStillStopped(t *testing.T) { + w := replay293(t, map[string]string{"build-agent": matched}) + id := IDOf(ctl474, head293) + if s := w.state(id); s != Published { + t.Fatalf("within the grace it is %s", s) + } + w.later(strandedAfter + time.Minute) + w.settleAll() + d := w.delivery(id) + if d.State != Stopped { + t.Fatalf("route-proxy neither built nor unchanged, it is %s", d.State) + } + why := d.Transitions[len(d.Transitions)-1].Why + if !strings.Contains(why, "without building route-proxy (") { + t.Fatalf("its stop says %q", why) + } +} + +// A walk that says nothing unchanged — a controller from before, or one that built nothing at all — stops it as +// before (issue 474). +func TestAMergeWhoseWalkSaysNothingUnchangedIsStillStopped(t *testing.T) { + w := replay293(t, nil) + w.later(strandedAfter + time.Minute) + w.settleAll() + if d := w.delivery(IDOf(ctl474, head293)); d.State != Stopped || + !strings.Contains(d.Stranded, "without building build-agent, route-proxy") { + t.Fatalf("it is %s: %q", d.State, d.Stranded) + } +} + +// Batched behind a later merge of its branch (issue 474's shape), a merge whose module the walk said unchanged moves +// nothing at either commit: delivered, not covered by a later merge that delivered nothing for it. +func TestABatchedMergeWhoseModuleMatchesWhatRunsIsDelivered(t *testing.T) { + w := newWorldOver(t, newMemStore(), newFakeController(), newFakeForge(), at215.Add(-time.Minute)) + p215 := pr(ctl474, 215, head215, "fix/431-make-check-runs-what-the-gate-runs", "Makefile") + p217 := pr(ctl474, 217, head217, "fix/432-a-test-reads-what-it-carries", "cmd/mesh-controller/check_test.go") + w.h.PullUpdated(p215) + w.h.PullUpdated(p217) + w.h.Checked(verdictMoving(ctl474, 215, head215, "build-agent")) + w.h.Checked(verdictMoving(ctl474, 217, head217)) + w.later(time.Minute) + w.h.PullMerged(mergedAt(p215, merge215, at215)) + w.later(time.Minute) + w.h.PullMerged(mergedAt(p217, merge217, at215.Add(56*time.Second))) + w.later(90 * time.Second) + w.h.WalkMoved(w.ctl.put(Walk{ID: batchWalk, Repository: ctl474, Branch: "main", Commit: merge217, Created: w.now, + Updated: w.now, State: walkDone, Tiers: [][]string{}, Modules: map[string]*WalkModule{}, Note: nothingNote, + Delivery: &WalkDelivery{Awaits: "mesh-delivery", Merges: []WalkMerge{ + {Repository: ctl474, Commit: merge215, Carried: merge217}, {Repository: ctl474, Commit: merge217}}, + Unchanged: map[string]string{"build-agent": "its build source holds none of the 2 changed file(s)"}}})) + w.settleAll() + if d := w.delivery(IDOf(ctl474, head215)); d.State != Delivered { + t.Fatalf("#215, build-agent said unchanged by its batch's walk, is %s, waiting for %q", d.State, w.h.waitsFor(d)) + } +} + +func sortedKeys(m map[string]string) []string { + var out []string + for k := range m { + out = append(out, k) + } + sort.Strings(out) + return out +} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/model.go b/modules/mesh-delivery/cmd/mesh-delivery/model.go index 37f0957b..85045b5d 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/model.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/model.go @@ -3,6 +3,7 @@ package main import ( "encoding/json" "fmt" + "maps" "regexp" "sort" "strings" @@ -460,6 +461,11 @@ type WalkDelivery struct { // cut, is the walk under the same id: its next record is `building`. A merge this owner reads in a batch is // not held as unwalked while the batch is heard of. Batch *WalkBatch `json:"batch,omitempty"` + // Unchanged are the modules the walk did not build because their build source holds none of the files its + // merges changed, each with why (novox/hq issue 611, ADR 0267 §5): the build the mesh holds is theirs. A delivery + // plan that read them whole names them; this says they matched what runs, not that the walk left them unbuilt. + // Absent from a controller that does not say it. + Unchanged map[string]string `json:"unchanged,omitempty"` } // WalkMerge is one merge a walk answers. @@ -583,6 +589,9 @@ type WalkSeen struct { // from them. Moved []string `json:"moved,omitempty"` MovedSaid bool `json:"moved_said,omitempty"` + // Unchanged are the modules the walk said matched what runs, with why (novox/hq issue 611): not built, and + // nothing for it to deliver. + Unchanged map[string]string `json:"unchanged,omitempty"` } // Seen is a walk as a delivery keeps it. @@ -594,6 +603,9 @@ func (w Walk) Seen() *WalkSeen { s.Waits = d.Awaits != "" && d.Go == nil s.GoBy = d.By s.StoppedBy, s.StoppedWhy = d.Stopped, d.StoppedWhy + if len(d.Unchanged) > 0 { + s.Unchanged = maps.Clone(d.Unchanged) + } } s.Branch = w.Branch s.Created = w.Created @@ -644,9 +656,10 @@ func (d *Delivery) stoppedWords() string { return s } -// walkMovedItsModules is whether the walk it follows built every module its plan moves, and whether that is known: -// from the modules the walk's record names, or, for a record kept before they were, from a walk of no tier, which -// moved nothing. Unknown when its plan is not known. +// walkMovedItsModules is whether the walk it follows answered every module its plan moves, and whether that is +// known: each built, or said unchanged — its build source holds none of the files the walk's merges changed, so the +// build the mesh holds is its build (novox/hq issue 611) — from the modules the walk's record names, or, for a +// record kept before they were, from a walk of no tier, which moved nothing. Unknown when its plan is not known. func (d *Delivery) walkMovedItsModules() (moved, known bool) { if d.Walk == nil || d.Plan == nil { return false, false @@ -654,28 +667,46 @@ func (d *Delivery) walkMovedItsModules() (moved, known bool) { if !d.Walk.MovedSaid && d.Walk.Tiers > 0 { return false, false } - for _, m := range d.Plan.Moved { - if !containsString(d.Walk.Moved, m) { - return false, true - } - } - return true, true + return len(d.notMovedBy()) == 0, true } -// notMovedBy is the modules its plan moves that the walk it follows did not build. +// notMovedBy is the modules its plan moves that the walk it follows neither built nor said unchanged: what a walk +// that ended without building them left undelivered. func (d *Delivery) notMovedBy() []string { var out []string if d.Walk == nil || d.Plan == nil { return nil } for _, m := range d.Plan.Moved { - if !containsString(d.Walk.Moved, m) { + if _, unchanged := d.Walk.Unchanged[m]; !containsString(d.Walk.Moved, m) && !unchanged { out = append(out, m) } } return out } +// unchangedOfIts is the modules its plan moves that the walk it follows said matched what runs, sorted (novox/hq +// issue 611). +func (d *Delivery) unchangedOfIts() []string { + var out []string + if d.Walk == nil || d.Plan == nil { + return nil + } + for _, m := range d.Plan.Moved { + if _, unchanged := d.Walk.Unchanged[m]; unchanged && !containsString(d.Walk.Moved, m) { + out = append(out, m) + } + } + sort.Strings(out) + return out +} + +// builtNoneOfIts is whether the walk it follows built none of the modules its plan moves and said each unchanged: +// its merge moves nothing, whichever commit the walk carried (ADR 0270 §1). +func (d *Delivery) builtNoneOfIts() bool { + return d.Plan != nil && len(d.Plan.Moved) > 0 && len(d.unchangedOfIts()) == len(d.Plan.Moved) +} + // Started is whether the walk is past its wait: it waited for nobody, or was let go — whatever came of it // since. A walk superseded or stopped while it waited never started. func (s *WalkSeen) Started() bool { return s != nil && !s.Waits && (s.WaitedFor == "" || s.GoBy != "") } diff --git a/modules/mesh-delivery/cmd/mesh-delivery/table.go b/modules/mesh-delivery/cmd/mesh-delivery/table.go index 04fa603f..687f1b2c 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/table.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/table.go @@ -249,19 +249,31 @@ var Table = []Row{ }}, {From: []State{Published}, Event: EvDone, To: Delivered, Guard: "nothing for a walk to move; or its walk, carrying its own commit, ended done before its word having built " + - "every module it moves (novox/hq issue 474)", + "every module it moves or said it unchanged (novox/hq issue 474); or its walk said every module it moves " + + "unchanged, its build source holding none of the files changed, so it moves nothing (novox/hq issue 611, " + + "ADR 0270 §1)", Holds: func(d *Delivery, f Facts) error { if d.Walk == nil && d.MovesNothing() { return nil } moved, _ := d.walkMovedItsModules() - return unless(d.endedUnstarted() && !d.carriedLater() && moved, "a walk moves it") + return unless(d.endedUnstarted() && moved && (!d.carriedLater() || d.builtNoneOfIts()), "a walk moves it") }, Says: func(d *Delivery) string { if d.Walk == nil { return "nothing for a walk to move" } - return fmt.Sprintf("its walk %s ended done before its word, having built every module it moves", d.Walk.ID) + unchanged := d.unchangedOfIts() + switch { + case len(unchanged) == 0: + return fmt.Sprintf("its walk %s ended done before its word, having built every module it moves", d.Walk.ID) + case d.builtNoneOfIts(): + return fmt.Sprintf("its walk %s moved nothing: %s matched what runs, its build source holding none of "+ + "the files changed, so the build the mesh holds is its build (novox/hq issue 611)", d.Walk.ID, + strings.Join(unchanged, ", ")) + } + return fmt.Sprintf("its walk %s ended done before its word, having built every module it moves but %s, "+ + "which matched what runs (novox/hq issue 611)", d.Walk.ID, strings.Join(unchanged, ", ")) }}, {From: []State{Published, Ready}, Event: EvHold, To: Held, Guard: "something holds it for a person: its group's composed check did not pass for the heads that merged, " +