From 14f405ece5088b300cf612d5b562efa62b6e0c9d Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 19 Oct 2026 16:42:53 +0200 Subject: [PATCH] Announce a plan from a copy that shares nothing with the walk, which goes on writing its steps (issue 1260) --- cmd/mesh-controller/delivery.go | 26 ++++- .../plan_announce_copy_test.go | 102 ++++++++++++++++++ 2 files changed, 127 insertions(+), 1 deletion(-) create mode 100644 cmd/mesh-controller/plan_announce_copy_test.go diff --git a/cmd/mesh-controller/delivery.go b/cmd/mesh-controller/delivery.go index 87325a48..d8590a02 100644 --- a/cmd/mesh-controller/delivery.go +++ b/cmd/mesh-controller/delivery.go @@ -1220,8 +1220,32 @@ func shelfOf(entries []inventory.Entry) map[string]catalogue.Manifest { // On the save that starts the walk and the one that ends it, the walk also says what it touches, its bound and, // at the end, its outcome (novox/hq ADR 0308 decision 1, walk_said.go), read from facts; nil facts say only what // the walk recorded. +// +// The plan the announcement is made from is a copy that shares nothing with the walk, taken before the goroutine +// starts: the walk goes on writing its steps' maps and pointers while the announcement is marshalled and its +// touches read (novox/hq issue 1260), and it says the plan as it was when this was called. func sayPlanMoved(ctx context.Context, bus link.Bus, facts walkFacts, p inventory.Plan) { - go publishPlanMoved(ctx, bus, facts, p) + kept, err := copyOfPlan(p) + if err != nil { + fmt.Fprintf(os.Stderr, "%s: kept, and could not be said as plan-moved: %v\n", p.ID, err) + return + } + go publishPlanMoved(ctx, bus, facts, kept) +} + +// copyOfPlan is the plan with nothing shared with the original: its JSON round trip, which is all of it but what +// the plan never keeps (WalkSaid), carried over. +func copyOfPlan(p inventory.Plan) (inventory.Plan, error) { + raw, err := json.Marshal(p) + if err != nil { + return inventory.Plan{}, err + } + var kept inventory.Plan + if err := json.Unmarshal(raw, &kept); err != nil { + return inventory.Plan{}, err + } + kept.WalkSaid = p.WalkSaid + return kept, nil } func publishPlanMoved(ctx context.Context, bus link.Bus, facts walkFacts, p inventory.Plan) { diff --git a/cmd/mesh-controller/plan_announce_copy_test.go b/cmd/mesh-controller/plan_announce_copy_test.go new file mode 100644 index 00000000..66c0a3d0 --- /dev/null +++ b/cmd/mesh-controller/plan_announce_copy_test.go @@ -0,0 +1,102 @@ +package main + +import ( + "context" + "encoding/json" + "reflect" + "testing" + "time" + + "github.com/novox/mesh-controller/internal/inventory" + "github.com/novox/mesh-controller/internal/link" +) + +// heldPublish is a bus that holds a plan-moved publish until it is let go, and keeps the body. +type heldPublish struct { + link.Bus + let chan struct{} + body chan []byte +} + +func (h *heldPublish) PublishSeatEvent(_ context.Context, _, _ string, body []byte) error { + <-h.let + h.body <- body + return nil +} + +func aWalkWithSteps() inventory.Plan { + built := time.Date(2026, 10, 19, 12, 0, 0, 0, time.UTC) + return inventory.Plan{ID: "plan-1", Repository: "novox/one", Commit: "abc", State: "walking", Tier: 1, + Created: built, Updated: built, Revision: 4, + Tiers: [][]string{{"alpha"}, {"beta", "gamma"}}, + Modules: map[string]*inventory.PlanModule{ + "alpha": {State: "built", BuiltAt: &built, Commit: "abc", First: []string{"ace"}}, + "beta": {State: "asked", Why: "waiting"}, + "gamma": {}, + }, + Times: &inventory.PlanTimes{Started: &built}, WalkSaid: inventory.WalkStarted} +} + +func TestAnAnnouncementIsThePlanAsItWasWhenSaidWhatTheWalkDoesNext(t *testing.T) { // 1260/AC1 + p := aWalkWithSteps() + bus := &heldPublish{let: make(chan struct{}), body: make(chan []byte, 1)} + sayPlanMoved(t.Context(), bus, nil, p) + + // The walk goes on, writing its steps while the announcement is being made. + for i := 0; i < 2000; i++ { + step := p.Modules["beta"] + step.State, step.Why = "built", "changed after the call" + step.First = append(step.First[:0], "novox") + p.Modules["delta"] = &inventory.PlanModule{State: "asked"} + p.Tiers[1][0] = "changed" + p.Times.Started = nil + delete(p.Modules, "delta") + } + close(bus.let) + + var said inventory.Plan + select { + case body := <-bus.body: + if err := json.Unmarshal(body, &said); err != nil { + t.Fatal(err) + } + case <-time.After(10 * time.Second): + t.Fatal("nothing was announced") + } + if said.Modules["beta"].State != "asked" || said.Modules["beta"].Why != "waiting" || said.Tiers[1][0] != "beta" || + len(said.Modules) != 3 || said.Times == nil || said.Times.Started == nil { + t.Errorf("the announcement carries the walk as it went on, not as it was: %+v", said) + } +} + +func TestAnAnnouncementOfAnUnchangedPlanIsTheBodyItWasBeforeTheCopy(t *testing.T) { // 1260/AC2 + p := aWalkWithSteps() + bus := &heldPublish{let: make(chan struct{}), body: make(chan []byte, 1)} + close(bus.let) + sayPlanMoved(t.Context(), bus, nil, p) + var got []byte + select { + case got = <-bus.body: + case <-time.After(10 * time.Second): + t.Fatal("nothing was announced") + } + // What it was: the plan whole, its phases worked out, and what the walk says of itself. + q := p + q.Phases = q.WalkPhases(time.Now(), nil) + want, err := json.Marshal(planMovedBody{Plan: q, walkSaid: sayWalk(t.Context(), nil, q)}) + if err != nil { + t.Fatal(err) + } + var a, b map[string]any + if json.Unmarshal(got, &a) != nil || json.Unmarshal(want, &b) != nil { + t.Fatal("a body is not JSON") + } + delete(a, "phases") // worked out from the time of the call + delete(b, "phases") + if !reflect.DeepEqual(a, b) { + t.Errorf("the body changed:\n got %s\nwant %s", got, want) + } + if a["walk"] != "plan-1" { + t.Errorf("the walk's own words are not in the body: %s", got) + } +} -- 2.54.0