From cd6610cdf792bbe37f90883bd24bd166ca55b1b3 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 12 Oct 2026 12:08:34 +0200 Subject: [PATCH] Say a walk's start or end when a person stops, closes, retries or releases it, and a hand stop as stopped plans stop|close, plans retry and upgrade release-backlog kept a walk's start or end mark without saying it, so the router would hold what the walk touched until its bound; each now runs on the bus as plans go does. A hand stop keeps who stopped it, so its outcome is stopped, not failed (review 15, ADR 0308, issue 532). --- cmd/mesh-controller/release.go | 19 ++-- cmd/mesh-controller/release_plan.go | 64 +++++++---- cmd/mesh-controller/walk_said_test.go | 157 ++++++++++++++++++++------ internal/inventory/plans.go | 4 +- testdata/store-tests | 3 + 5 files changed, 180 insertions(+), 67 deletions(-) diff --git a/cmd/mesh-controller/release.go b/cmd/mesh-controller/release.go index 555ebeb2..bc16fd84 100644 --- a/cmd/mesh-controller/release.go +++ b/cmd/mesh-controller/release.go @@ -812,14 +812,19 @@ func backlogCommand(ctx context.Context, sub string, args []string) error { } defer open.Close() if sub == "release-backlog" { - release, err := open.inventory.HoldPlans(ctx, true) - if err != nil { + // Said on the bus (novox/hq ADR 0308 decision 1): the release plan starts walking here, and its start is + // said by the verb that opens it. + var p *inventory.Plan + if err := sayingOnTheBus(ctx, open.inventory, func() error { + release, err := open.inventory.HoldPlans(ctx, true) + if err != nil { + return err + } + defer release() + why.record(ctx, "upgrade release-backlog", nil) + p, err = releaseBacklog(ctx, open, link.Caller()) return err - } - defer release() - why.record(ctx, "upgrade release-backlog", nil) - p, err := releaseBacklog(ctx, open, link.Caller()) - if err != nil { + }); err != nil { return err } if p == nil { diff --git a/cmd/mesh-controller/release_plan.go b/cmd/mesh-controller/release_plan.go index 504f0b0a..1710c59a 100644 --- a/cmd/mesh-controller/release_plan.go +++ b/cmd/mesh-controller/release_plan.go @@ -1571,8 +1571,13 @@ func plansCommand(ctx context.Context, args []string) error { } // `retry` (novox/hq ADR 0219): a failed plan's failed builds asked again, and the plan goes on. if len(positionals) == 2 && positionals[0] == "retry" { - said, err := retryPlan(ctx, open, positionals[1]) - if err != nil { + // Said on the bus (novox/hq ADR 0308 decision 1): a retried walk starts again, and its start is said + // by the verb that starts it. + var said string + if err := sayingOnTheBus(ctx, inv, func() (err error) { + said, err = retryPlan(ctx, open, positionals[1]) + return err + }); err != nil { return err } fmt.Println(said) @@ -1586,26 +1591,29 @@ func plansCommand(ctx context.Context, args []string) error { if positionals[0] == "close" { how = "closed" } - release, err := inv.HoldPlans(ctx, true) - if err != nil { - return err - } - defer release() - p, err := inv.PlanByID(ctx, positionals[1]) - if err != nil { - return err - } - if !p.Open() { - return fmt.Errorf("%s is already %s", p.ID, p.State) - } - why.record(ctx, "plans "+positionals[0], positionals[1:]) - p.State = inventory.PlanFailed - p.Note = how + " by hand at tier " + fmt.Sprint(p.Tier) + ": " + strings.TrimSpace(*why.why) - sayUnsent(&p, func(m string) bool { - u, err := inv.UpgradeOf(ctx, m) - return err == nil && u.RollOut - }) - if err := inv.SavePlan(ctx, &p); err != nil { + // Said on the bus (novox/hq ADR 0308 decision 1): the walk's end, outcome stopped, is said by the verb + // that ends it, so what it touched is not held until its bound. + var p inventory.Plan + if err := sayingOnTheBus(ctx, inv, func() error { + release, err := inv.HoldPlans(ctx, true) + if err != nil { + return err + } + defer release() + if p, err = inv.PlanByID(ctx, positionals[1]); err != nil { + return err + } + if !p.Open() { + return fmt.Errorf("%s is already %s", p.ID, p.State) + } + why.record(ctx, "plans "+positionals[0], positionals[1:]) + stopByHand(&p, how, "a person ("+whoAsked()+")", strings.TrimSpace(*why.why)) + sayUnsent(&p, func(m string) bool { + u, err := inv.UpgradeOf(ctx, m) + return err == nil && u.RollOut + }) + return inv.SavePlan(ctx, &p) + }); err != nil { return err } fmt.Printf("%s %s at tier %d of %d; what was asked still builds and registers, nothing further is asked\n", @@ -1924,3 +1932,15 @@ func forgetDeleted(ctx context.Context, inv *inventory.Inventory, module string, "`module forget %s`: %v\n", p.ID, module, module, firstLine(err.Error())) } } + +// stopByHand ends a walk a person stopped or closed (`plans stop|close`): failed, with who stopped it and why +// kept as the delivery's owner keeps its own stop, so the walk ends with outcome stopped, not failed (novox/hq +// ADR 0308 decision 1). +func stopByHand(p *inventory.Plan, how, by, why string) { + p.State = inventory.PlanFailed + p.Note = how + " by hand at tier " + fmt.Sprint(p.Tier) + ": " + why + if p.Delivery == nil { + p.Delivery = &inventory.PlanDelivery{} + } + p.Delivery.Stopped, p.Delivery.StoppedWhy = by, why +} diff --git a/cmd/mesh-controller/walk_said_test.go b/cmd/mesh-controller/walk_said_test.go index 70a4aaac..a5075508 100644 --- a/cmd/mesh-controller/walk_said_test.go +++ b/cmd/mesh-controller/walk_said_test.go @@ -236,24 +236,7 @@ func TestPlanMovedCarriesTheWalkWholeAndOnItsStartWhatItTouches(t *testing.T) { func TestAWalkLetGoAndStoppedByVerbsIsSaidStartedAndEnded(t *testing.T) { open := aMesh(t) ctx := t.Context() - url := testbus.URL(t) - js, err := broker.Dial(url) - if err != nil { - t.Fatal(err) - } - t.Cleanup(js.Close) - if err := broker.AssertMeshStreams(js); err != nil { - t.Fatal(err) - } - before := handActConn - handActConn = js.Conn() - t.Cleanup(func() { handActConn = before }) - heard := make(chan *nats.Msg, 8) - sub, err := js.Conn().ChanSubscribe(link.SeatEventSubject(link.MeshControllerSeat, link.KeyPlanMoved), heard) - if err != nil { - t.Fatal(err) - } - t.Cleanup(func() { _ = sub.Unsubscribe() }) + next := planMovedHeard(t) register(t, open, catalogue.Manifest{Module: "app", Version: "1"}) if _, err := assign(ctx, open, "anchor", "app"); err != nil { t.Fatal(err) @@ -267,24 +250,6 @@ func TestAWalkLetGoAndStoppedByVerbsIsSaidStartedAndEnded(t *testing.T) { if err := open.inventory.SavePlan(ctx, &waiting); err != nil { t.Fatal(err) } - next := func(phase string) map[string]any { - t.Helper() - deadline := time.After(5 * time.Second) - for { - select { - case m := <-heard: - var said map[string]any - if err := json.Unmarshal(m.Data, &said); err != nil { - t.Fatal(err) - } - if said["phase"] == phase { - return said - } - case <-deadline: - t.Fatalf("the walk was not said %s", phase) - } - } - } if err := deliveryCommand(ctx, []string{"go", waiting.ID, "--why", "its turn"}); err != nil { t.Fatal(err) } @@ -304,3 +269,123 @@ func TestAWalkLetGoAndStoppedByVerbsIsSaidStartedAndEnded(t *testing.T) { t.Fatalf("the end, which sent nothing: %v", ended) } } + +// planMovedHeard raises a bus of the test's own, points the verbs' hand-act connection at it, and returns what +// waits for the next plan-moved said with the given phase, failing the test when none comes within 5 s. +func planMovedHeard(t *testing.T) func(phase string) map[string]any { + t.Helper() + js, err := broker.Dial(testbus.URL(t)) + if err != nil { + t.Fatal(err) + } + t.Cleanup(js.Close) + if err := broker.AssertMeshStreams(js); err != nil { + t.Fatal(err) + } + before := handActConn + handActConn = js.Conn() + t.Cleanup(func() { handActConn = before }) + heard := make(chan *nats.Msg, 64) + sub, err := js.Conn().ChanSubscribe(link.SeatEventSubject(link.MeshControllerSeat, link.KeyPlanMoved), heard) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = sub.Unsubscribe() }) + return func(phase string) map[string]any { + t.Helper() + deadline := time.After(5 * time.Second) + for { + select { + case m := <-heard: + var said map[string]any + if err := json.Unmarshal(m.Data, &said); err != nil { + t.Fatal(err) + } + if said["phase"] == phase { + return said + } + case <-deadline: + t.Fatalf("no walk was said %s", phase) + } + } + } +} + +// Live, with the store and a bus: a walk a person stops with `plans stop` is said ended, outcome stopped, by the +// verb itself, and is kept stopped by that person; `plans close` ends one the same way (novox/hq ADR 0308 +// decision 1, review 15 on mesh-controller#247). +func TestAWalkStoppedOrClosedByAPersonIsSaidEndedStopped(t *testing.T) { + open := aMesh(t) + ctx := t.Context() + next := planMovedHeard(t) + register(t, open, catalogue.Manifest{Module: "app", Version: "1"}) + if _, err := assign(ctx, open, "anchor", "app"); err != nil { + t.Fatal(err) + } + for _, verb := range []string{"stop", "close"} { + cut := time.Now().UTC() + walk := inventory.Plan{ID: "plan-308-" + verb, Repository: "novox/mesh-catalog", Branch: "main", + Commit: "c8c8c8c8", Created: cut, State: inventory.PlanBuilding, Tiers: [][]string{{"app"}}, + Modules: map[string]*inventory.PlanModule{"app": {}}, + Times: &inventory.PlanTimes{Cut: &cut, Started: &cut, Class: inventory.ClassLeaf}} + if err := open.inventory.SavePlan(ctx, &walk); err != nil { + t.Fatal(err) + } + if err := plansCommand(ctx, []string{verb, walk.ID, "--why", "a test"}); err != nil { + t.Fatal(err) + } + ended := next(inventory.WalkEnded) + if ended["walk"] != walk.ID || ended["outcome"] != inventory.WalkStopped { + t.Fatalf("plans %s: the end is %v", verb, ended) + } + p, err := open.inventory.PlanByID(ctx, walk.ID) + if err != nil { + t.Fatal(err) + } + if p.Outcome() != inventory.WalkStopped || p.Delivery == nil || p.Delivery.StoppedWhy != "a test" { + t.Fatalf("plans %s kept the walk %s, %+v", verb, p.Outcome(), p.Delivery) + } + } +} + +// Live, with the store and a bus: a failed walk retried with `plans retry` starts again, and the verb says its +// start (novox/hq ADR 0308 decision 1, review 15 on mesh-controller#247). +func TestAWalkRetriedByAPersonIsSaidStartedAgain(t *testing.T) { + open := aMesh(t) + ctx := t.Context() + asksRecorded(t) + next := planMovedHeard(t) + twoTiers(t, open) + before := time.Now().UTC().Add(-time.Hour) + ended := before.Add(time.Minute) + failed := inventory.Plan{ID: "plan-308-retry", Repository: "novox/a", Branch: "main", Commit: "c0ffee", + Created: before, State: inventory.PlanFailed, Tier: 0, Tiers: [][]string{{"a"}, {"b"}}, + Note: "a failed to build in tier 0", + Modules: map[string]*inventory.PlanModule{"a": {State: "failed", AskedAt: &before, Build: "build-1", + Why: link.KilledByHand}}, + Times: &inventory.PlanTimes{Cut: &before, Started: &before, Ended: &ended, Class: inventory.ClassLeaf}} + if err := open.inventory.SavePlan(ctx, &failed); err != nil { + t.Fatal(err) + } + if err := plansCommand(ctx, []string{"retry", failed.ID}); err != nil { + t.Fatal(err) + } + started := next(inventory.WalkStarted) + if started["walk"] != failed.ID || started["budget_class"] != inventory.ClassLeaf { + t.Fatalf("the retried start: %v", started) + } +} + +// Live, with the store and a bus: the release plan `upgrade release-backlog` opens is said started by the verb +// (novox/hq ADR 0308 decision 1, review 15 on mesh-controller#247). +func TestABacklogReleasedByAPersonIsSaidStarted(t *testing.T) { + b := aBacklog(t) + next := planMovedHeard(t) + if err := backlogCommand(t.Context(), "release-backlog", []string{"--why", "a test"}); err != nil { + t.Fatal(err) + } + started := next(inventory.WalkStarted) + if p := b.release(t); started["walk"] != p.ID { + t.Fatalf("the release's start: %v", started) + } +} diff --git a/internal/inventory/plans.go b/internal/inventory/plans.go index d044d29a..a0ec1bf8 100644 --- a/internal/inventory/plans.go +++ b/internal/inventory/plans.go @@ -191,8 +191,8 @@ type PlanDelivery struct { Go *time.Time `json:"go,omitempty"` By string `json:"by,omitempty"` Why string `json:"why,omitempty"` - // Stopped is who ended the walk through the delivery's owner, and StoppedWhy why: the walk is failed, - // and the delivery reads it as stopped, not as a build that failed. + // Stopped is who ended the walk, through the delivery's owner or by hand (`plans stop|close`), and StoppedWhy + // why: the walk is failed, and the delivery reads it as stopped, not as a build that failed. Stopped string `json:"stopped,omitempty"` StoppedWhy string `json:"stopped_why,omitempty"` // Merges is every merge the walk or batch answers (novox/hq ADR 0276): its own commits, and the earlier diff --git a/testdata/store-tests b/testdata/store-tests index 729b6261..8b270fb1 100644 --- a/testdata/store-tests +++ b/testdata/store-tests @@ -16,3 +16,6 @@ ./cmd/mesh-controller TestTheConditionsVerbReadsTheLooksTheBucketNoLongerHolds ./internal/inventory TestAWalksStartIsKeptAndARefusedSaveLeavesItUnsaid ./cmd/mesh-controller TestAWalkLetGoAndStoppedByVerbsIsSaidStartedAndEnded +./cmd/mesh-controller TestAWalkStoppedOrClosedByAPersonIsSaidEndedStopped +./cmd/mesh-controller TestAWalkRetriedByAPersonIsSaidStartedAgain +./cmd/mesh-controller TestABacklogReleasedByAPersonIsSaidStarted