From 66d22dd53eff09c25ffa0ad46657d66035a86e0b Mon Sep 17 00:00:00 2001 From: jochen Date: Tue, 13 Oct 2026 23:04:41 +0200 Subject: [PATCH] Let a closed pull request's check builds go, and show what each walk build did MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A check's builds for its merge have no taker once the pull request closes unmerged, and the delivery should say whether a module was reused from the check or built, and why (novox/hq ADR 0350 §4, §5). --- .../cmd/mesh-delivery/adr350_test.go | 50 +++++++++++++++++++ .../cmd/mesh-delivery/fakes_test.go | 12 +++++ .../mesh-delivery/cmd/mesh-delivery/holder.go | 15 +++++- .../cmd/mesh-delivery/manifest_test.go | 2 +- .../mesh-delivery/cmd/mesh-delivery/model.go | 12 +++++ .../mesh-delivery/cmd/mesh-delivery/ports.go | 8 +++ modules/mesh-delivery/module.json | 1 + 7 files changed, 98 insertions(+), 2 deletions(-) create mode 100644 modules/mesh-delivery/cmd/mesh-delivery/adr350_test.go diff --git a/modules/mesh-delivery/cmd/mesh-delivery/adr350_test.go b/modules/mesh-delivery/cmd/mesh-delivery/adr350_test.go new file mode 100644 index 00000000..05075466 --- /dev/null +++ b/modules/mesh-delivery/cmd/mesh-delivery/adr350_test.go @@ -0,0 +1,50 @@ +package main + +import ( + "reflect" + "strings" + "testing" + "time" +) + +// One build per change (novox/hq ADR 0350): a pull request closed unmerged has its check builds let go of, and a +// walk's record says per module whether its build reused a check's build or built, and why. + +// A pull request closed unmerged asks the controller to let go of what its checks built for its merge (§5); one +// the controller cannot answer is said, never fatal. +func TestAPullRequestClosedUnmergedLetsGoOfItsCheckBuilds(t *testing.T) { + w := newWorldOver(t, newMemStore(), newFakeController(), newFakeForge(), atCC) + cc := pr(cat, 101, headCC, "feat/claude-code", "modules/claude-code/module.json") + w.h.PullUpdated(cc) + w.h.PullClosed(cc) + if want := []string{"novox/mesh-catalog#101"}; !reflect.DeepEqual(w.ctl.forgot, want) { + t.Fatalf("closed unmerged, the controller was asked to let go of %v", w.ctl.forgot) + } + if s := w.state(IDOf(cat, headCC)); s != Stopped { + t.Fatalf("closed unmerged it is %s", s) + } + + w.ctl.down = true + du := pr(cat, 102, headDu, "feat/dunst", "modules/dunst/module.json") + w.h.PullUpdated(du) + w.h.PullClosed(du) + if s := w.state(IDOf(cat, headDu)); s != Stopped { + t.Fatalf("with the controller down, a closed delivery is %s", s) + } +} + +// The walk's record says per module what its build did, and the delivery keeps it beside its walk (§4). +func TestAWalksRecordSaysWhetherEachModuleWasReusedOrBuilt(t *testing.T) { + walk := catWalk("plan-1", mergeCC, atCC.Add(time.Minute), "claude-code", "dunst") + walk.Modules["claude-code"].Made = "reused from the check of novox/mesh-catalog#101 at head cccccccccccc " + + "(merged into trunk 553b71910000): tools sha256:aa" + walk.Modules["dunst"].Made = "built: no check of novox/mesh-catalog#101 built it" + seen := walk.Seen() + if !strings.HasPrefix(seen.Built["claude-code"], "reused from the check of novox/mesh-catalog#101") || + seen.Built["dunst"] != "built: no check of novox/mesh-catalog#101 built it" { + t.Fatalf("the delivery keeps %v", seen.Built) + } + if catWalk("plan-2", mergeCC, atCC, "dunst").Seen().Built != nil { + t.Fatal("a walk from a controller that says nothing of its builds reads as if it said something") + } +} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go b/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go index 4c1b703d..a9f0768a 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go @@ -110,6 +110,18 @@ type fakeController struct { onCheck func() // planned is what Plan answers instead of "builds app", when set. planned *DeliveryPlan + // forgot are the pull requests whose check builds were let go of (novox/hq ADR 0350 §5), "owner/repo#n". + forgot []string +} + +func (c *fakeController) ForgetChecks(owner, repo string, number int) error { + c.mu.Lock() + defer c.mu.Unlock() + if c.down { + return errors.New("down") + } + c.forgot = append(c.forgot, fmt.Sprintf("%s/%s#%d", owner, repo, number)) + return nil } func newFakeController() *fakeController { diff --git a/modules/mesh-delivery/cmd/mesh-delivery/holder.go b/modules/mesh-delivery/cmd/mesh-delivery/holder.go index 81c8279d..45dcaae3 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/holder.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/holder.go @@ -593,9 +593,12 @@ func verdictOfState(s string) string { } // PullClosed takes a pull request closed unmerged: its open deliveries are stopped. +// +// **And what its checks built for its merge goes** (novox/hq ADR 0350 §5): nothing will take it now. Asked of the +// controller once the deliveries are settled, outside the lock; a refusal is said, and the build seat's holders let +// go of it within the week anyway. func (h *Holder) PullClosed(p PullEvent) { h.mu.Lock() - defer h.mu.Unlock() for _, d := range h.deliveries { if d.Repository == p.repository() && d.Number == p.Number && !d.State.Final() && d.MergedAs == "" { d.ClosedUnmerged = true @@ -605,6 +608,16 @@ func (h *Holder) PullClosed(p PullEvent) { } } } + h.mu.Unlock() + if h.Controller == nil || p.Owner == "" || p.Repo == "" || p.Number <= 0 { + return + } + if err := h.Controller.ForgetChecks(p.Owner, p.Repo, p.Number); err != nil { + h.mu.Lock() + h.refuse(fmt.Errorf("%s#%d closed, and its check builds stay until the week's sweep: %w", p.repository(), + p.Number, err)) + h.mu.Unlock() + } } // CheckedEvent is the controller's `checked`. diff --git a/modules/mesh-delivery/cmd/mesh-delivery/manifest_test.go b/modules/mesh-delivery/cmd/mesh-delivery/manifest_test.go index d5d819f0..6b4800f7 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/manifest_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/manifest_test.go @@ -61,7 +61,7 @@ func TestItClaimsTheDeliverySeatAndFollowsWhatItHandles(t *testing.T) { } // It calls exactly what its ports ask: the controller's six verbs and the forge's four tools. want := []string{} - for _, v := range []string{"delivery-plan", "delivery-order", "delivery-check", "deliver", "delivery-stop", "delivery-walks"} { + for _, v := range []string{"delivery-plan", "delivery-order", "delivery-check", "deliver", "delivery-stop", "delivery-walks", "forget-checks"} { want = append(want, "seat:"+ControllerSeat+"."+v) } want = append(want, "gitea.gitea_note_append", "gitea.gitea_delivery_view", "gitea.gitea_commit_status", diff --git a/modules/mesh-delivery/cmd/mesh-delivery/model.go b/modules/mesh-delivery/cmd/mesh-delivery/model.go index 85045b5d..1f25263b 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/model.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/model.go @@ -402,6 +402,9 @@ type WalkModule struct { Build string `json:"build,omitempty"` Gate *WalkGate `json:"gate,omitempty"` GatedBy string `json:"gated_by,omitempty"` + // Made is what the walk's build of it did (novox/hq ADR 0350 §4): "reused from ", naming the pull + // request's head, the trunk commit and the digests, or "built: ". Empty from a controller before it. + Made string `json:"made,omitempty"` } // WalkGate is a module's judging on its first machine. @@ -592,6 +595,9 @@ type WalkSeen struct { // 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"` + // Built is, per module, what the walk's build of it did (novox/hq ADR 0350 §4): "reused from " or + // "built: " — what `show` says beside the build phase. + Built map[string]string `json:"built,omitempty"` } // Seen is a walk as a delivery keeps it. @@ -616,6 +622,12 @@ func (w Walk) Seen() *WalkSeen { if m != nil && (m.SentAt != nil || m.Gate != nil && m.Gate.Verdict == "passed") { s.Moved = append(s.Moved, name) } + if m != nil && m.Made != "" { + if s.Built == nil { + s.Built = map[string]string{} + } + s.Built[name] = m.Made + } } sort.Strings(s.Moved) s.MovedSaid = true diff --git a/modules/mesh-delivery/cmd/mesh-delivery/ports.go b/modules/mesh-delivery/cmd/mesh-delivery/ports.go index c0185b1d..a4727928 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/ports.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/ports.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "sort" + "strconv" "strings" stdio "git.novox.be/novox/mesh-sdk/go" @@ -70,6 +71,8 @@ type Controller interface { StopWithout(walk, why, by string, without []string) (StopAnswer, error) // Walks are the walks the controller keeps — one, given its id — and whether this seat has a holder on record. Walks(walk string) (bool, []Walk, error) + // ForgetChecks lets go of the builds a pull request's merge checks made for its merge (novox/hq ADR 0350 §5). + ForgetChecks(owner, repo string, number int) error } // Forge is what the forge's holder does for a delivery. @@ -271,6 +274,11 @@ func (c seatController) Deliver(walk, why string) error { return err } +func (c seatController) ForgetChecks(owner, repo string, number int) error { + _, err := c.verb("forget-checks", map[string]any{"owner": owner, "repo": repo, "number": strconv.Itoa(number)}) + return err +} + func (c seatController) Stop(walk, why, by string) error { _, err := c.verb("delivery-stop", map[string]any{"plan": walk, "why": why, "by": by}) return err diff --git a/modules/mesh-delivery/module.json b/modules/mesh-delivery/module.json index 1215633c..681165fe 100644 --- a/modules/mesh-delivery/module.json +++ b/modules/mesh-delivery/module.json @@ -41,6 +41,7 @@ "seat:mesh-controller.deliver", "seat:mesh-controller.delivery-stop", "seat:mesh-controller.delivery-walks", + "seat:mesh-controller.forget-checks", "gitea.gitea_note_append", "gitea.gitea_delivery_view", "gitea.gitea_commit_status",