From facc551ff29baeac0357fdbc801308e56bdce54d Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 12 Oct 2026 11:54:35 +0200 Subject: [PATCH 1/3] Say what a walk touches, its bound and its outcome when it starts and ends (ADR 0308, issue 532) The router holds what an open walk touches and tells its result once; it can do so only if plan-moved names the walk's start and end, the modules and machines it moves, what a moved tool runner or node-engine carries there, and how long to hold. --- cmd/mesh-controller/delivery.go | 22 +- cmd/mesh-controller/push.go | 2 +- cmd/mesh-controller/release_plan.go | 2 +- cmd/mesh-controller/walk_said.go | 334 ++++++++++++++++++++++++++ cmd/mesh-controller/walk_said_test.go | 306 +++++++++++++++++++++++ internal/inventory/plans.go | 24 +- internal/inventory/walksaid.go | 81 +++++++ internal/inventory/walksaid_test.go | 130 ++++++++++ testdata/store-tests | 2 + 9 files changed, 891 insertions(+), 12 deletions(-) create mode 100644 cmd/mesh-controller/walk_said.go create mode 100644 cmd/mesh-controller/walk_said_test.go create mode 100644 internal/inventory/walksaid.go create mode 100644 internal/inventory/walksaid_test.go diff --git a/cmd/mesh-controller/delivery.go b/cmd/mesh-controller/delivery.go index 4055e730..23a0a629 100644 --- a/cmd/mesh-controller/delivery.go +++ b/cmd/mesh-controller/delivery.go @@ -940,7 +940,7 @@ func deliveryCommand(ctx context.Context, args []string) error { if *group == "" && len(members) == 1 { // One head, checked again as the forge's announcement would have it (a recheck): the controller's // own path for a pull request, run because the delivery's owner asked. - if err := sayingOnTheBus(ctx, func() error { return (following{open}).PullUpdated(ctx, members[0].pull()) }); err != nil { + if err := sayingOnTheBus(ctx, inv, func() error { return (following{open}).PullUpdated(ctx, members[0].pull()) }); err != nil { return err } return answer(map[string]any{"rechecked": members[0].ID}) @@ -958,7 +958,7 @@ func deliveryCommand(ctx context.Context, args []string) error { return errors.New("delivery go [--by ] [--why ]") } var p inventory.Plan - if err := sayingOnTheBus(ctx, func() (err error) { + if err := sayingOnTheBus(ctx, inv, func() (err error) { p, err = letGo(ctx, inv, positionals[0], *by, strings.TrimSpace(*why)) return err }); err != nil { @@ -973,7 +973,7 @@ func deliveryCommand(ctx context.Context, args []string) error { } var p inventory.Plan var said stopAnswer - if err := sayingOnTheBus(ctx, func() (err error) { + if err := sayingOnTheBus(ctx, inv, func() (err error) { p, said, err = stopWalkWithout(ctx, inv, positionals[0], *by, strings.TrimSpace(*why), splitList(*without), time.Now().UTC()) return err @@ -1215,15 +1215,19 @@ func shelfOf(entries []inventory.Entry) map[string]catalogue.Manifest { // delivery it walks reads its steps from. Never in the way of the walk: said beside it, and a save that could // not be said is logged — the delivery's owner, which also asks for the walks it follows, finds it by // comparison. Records arriving out of order are told apart by the plan's revision. -func sayPlanMoved(ctx context.Context, bus link.Bus, p inventory.Plan) { - go publishPlanMoved(ctx, bus, p) +// +// 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. +func sayPlanMoved(ctx context.Context, bus link.Bus, facts walkFacts, p inventory.Plan) { + go publishPlanMoved(ctx, bus, facts, p) } -func publishPlanMoved(ctx context.Context, bus link.Bus, p inventory.Plan) { +func publishPlanMoved(ctx context.Context, bus link.Bus, facts walkFacts, p inventory.Plan) { // Its phases so far (novox/hq ADR 0282 decision 6): the rest's reports come after the walk ends, and are // read by whoever asks for the walk (`delivery walks`). p.Phases = p.WalkPhases(time.Now(), nil) - body, err := json.Marshal(p) + body, err := json.Marshal(planMovedBody{Plan: p, walkSaid: sayWalk(ctx, facts, p)}) if err != nil { return } @@ -1238,7 +1242,7 @@ func publishPlanMoved(ctx context.Context, bus link.Bus, p inventory.Plan) { // controller's, or a connection of the command's own — a verb runs as a command of its own, and what it kept // would otherwise be said by nobody until the delivery's owner read the walks back. Without a bus the command // still runs; what it did is found by comparison. -func sayingOnTheBus(ctx context.Context, f func() error) error { +func sayingOnTheBus(ctx context.Context, facts walkFacts, f func() error) error { if checkEvents != nil { return f() } @@ -1250,7 +1254,7 @@ func sayingOnTheBus(ctx context.Context, f func() error) error { } bus := link.OverNATS{Conn: conn, JS: js} checkEvents = bus - inventory.PlanSaved = func(p inventory.Plan) { publishPlanMoved(ctx, bus, p) } + inventory.PlanSaved = func(p inventory.Plan) { publishPlanMoved(ctx, bus, facts, p) } defer func() { checkEvents, inventory.PlanSaved = nil, nil }() ran = true return f() diff --git a/cmd/mesh-controller/push.go b/cmd/mesh-controller/push.go index 812f6d79..f550fe03 100644 --- a/cmd/mesh-controller/push.go +++ b/cmd/mesh-controller/push.go @@ -249,7 +249,7 @@ func serve(ctx context.Context) (err error) { // (novox/hq to-be 45 §9). checkEvents = bus // And every walk this controller keeps in a new state, for the delivery it walks (novox/hq ADR 0239). - inventory.PlanSaved = func(p inventory.Plan) { sayPlanMoved(ctx, bus, p) } + inventory.PlanSaved = func(p inventory.Plan) { sayPlanMoved(ctx, bus, open.inventory, p) } if err := server.Checks(following{open}); err != nil { return err } diff --git a/cmd/mesh-controller/release_plan.go b/cmd/mesh-controller/release_plan.go index dcbd9a5d..504f0b0a 100644 --- a/cmd/mesh-controller/release_plan.go +++ b/cmd/mesh-controller/release_plan.go @@ -1559,7 +1559,7 @@ func plansCommand(ctx context.Context, args []string) error { if len(positionals) == 2 && positionals[0] == "go" { why.record(ctx, "plans go", positionals[1:]) var p inventory.Plan - if err := sayingOnTheBus(ctx, func() (err error) { + if err := sayingOnTheBus(ctx, inv, func() (err error) { p, err = letGo(ctx, inv, positionals[1], "a person ("+whoAsked()+")", strings.TrimSpace(*why.why)) return err }); err != nil { diff --git a/cmd/mesh-controller/walk_said.go b/cmd/mesh-controller/walk_said.go new file mode 100644 index 00000000..ddb724e4 --- /dev/null +++ b/cmd/mesh-controller/walk_said.go @@ -0,0 +1,334 @@ +package main + +import ( + "context" + "fmt" + "os" + "slices" + "sort" + "time" + + "github.com/novox/mesh-controller/internal/broker" + "github.com/novox/mesh-controller/internal/catalogue" + "github.com/novox/mesh-controller/internal/inventory" +) + +// What a walk touches, its bound and its outcome (novox/hq ADR 0308 decision 1, decision 5): said in +// `plan-moved` on the save that starts the walk and the save that ends it, beside the walk whole, for the holder +// of the operator's channel to hold what the walk touches while it runs and tell its result once. +// +// **What a walk touches** is every module it moves on every machine it sends it to, and every module a moved +// tool runner (node-tools) or node-engine (mesh-host) carries on that machine: what restarts there. At the start +// that is what the walk will send — each module of its tiers whose policy rolls it out, on every machine running +// it; at the end it is what it reached — the machines each module was sent to, first and rest, and what a gated +// send carried with it. +// +// **Its bound** is twice the largest delivery budget among the walk's deliveries (deliveryBudgets: a core +// module's ten minutes, a leaf module's budget in force), and fifteen minutes, not doubled, for a walk none of +// whose deliveries has a budget: a release plan (builds waiting for a person's gate), a walk whose every module's +// policy records rather than rolls out (held for a person, ADR 0282 decision 1), or a walk kept without a class. + +// walkNoBudgetBound is the bound of a walk with no budget (ADR 0308 decision 5): not doubled. +const walkNoBudgetBound = 15 * time.Minute + +// budgetNone is the budget class of a walk none of whose deliveries has a budget. +const budgetNone = "none" + +// walkSaid is what `plan-moved` adds on a walk's start and end, beside the walk whole. +type walkSaid struct { + Walk string `json:"walk"` + Phase string `json:"phase"` + // Touches is, per machine, the modules the walk moves or reaches there and what a moved tool runner or + // node-engine carries there; never null. + Touches []walkTouch `json:"touches"` + // Outcome is on the end only: delivered, failed or stopped. + Outcome string `json:"outcome,omitempty"` + // BoundSeconds and BudgetClass are said on both, read from the walk's class at the start. + BoundSeconds int `json:"bound_seconds"` + BudgetClass string `json:"budget_class"` + // StartedAt is when the walk was said started, on both; EndedAt when it was said ended, on the end: the + // bound runs from StartedAt, whenever the event is heard. + StartedAt *time.Time `json:"started_at,omitempty"` + EndedAt *time.Time `json:"ended_at,omitempty"` +} + +// walkTouch is what a walk touches on one machine. +type walkTouch struct { + Machine string `json:"machine"` + Modules []string `json:"modules"` + // Carried names those of Modules that are there only because a moved tool runner or node-engine carries + // them: what restarts with it, not what the walk builds. + Carried []string `json:"carried,omitempty"` +} + +// planMovedBody is `plan-moved`'s body: the walk whole, as before, and on its start and end what it touches. +type planMovedBody struct { + inventory.Plan + *walkSaid +} + +// walkFacts is what the touches are read from: the controller's inventory. +type walkFacts interface { + Running(ctx context.Context, module string) ([]string, error) + UpgradeOf(ctx context.Context, module string) (inventory.Upgrade, error) + Assigned(ctx context.Context, node string) ([]string, error) + SentBuilds(ctx context.Context, node string) (map[string]string, bool, error) + Catalogue(ctx context.Context) (map[string]catalogue.Manifest, error) +} + +// sayWalk is what `plan-moved` adds to the save that started or ended the walk; nil on every other save. A fact +// that cannot be read is said on the controller's log and left out, never in the way of the event: a walk said +// with fewer touches is held less, never longer. +func sayWalk(ctx context.Context, facts walkFacts, p inventory.Plan) *walkSaid { + if p.WalkSaid == "" { + return nil + } + w := &walkSaid{Walk: p.ID, Phase: p.WalkSaid} + if p.Times != nil { + w.StartedAt = p.Times.Started + if p.WalkSaid == inventory.WalkEnded { + w.EndedAt, w.Outcome = p.Times.Ended, p.Outcome() + } + } + read := readWalkFacts(ctx, facts, p) + w.BudgetClass = walkBudgetClass(p, read.rollsOut) + w.BoundSeconds = int(walkBound(w.BudgetClass) / time.Second) + var moved map[string][]string + if p.WalkSaid == inventory.WalkStarted { + moved = walkSends(p, read.running, read.rollsOut) + } else { + moved = walkReached(p, read.running) + } + var shelf map[string]catalogue.Manifest + if len(moved[broker.RuntimeModule]) > 0 { + // Which modules the tool runner serves is the catalogue's: read only when the walk moves it. + shelf = read.shelf() + } + w.Touches = touchesOf(moved, read.onMachine(moved), shelf) + return w +} + +// walkBudgetClass is the largest budget class among the walk's deliveries: its class read at the cut when one of +// its modules rolls out, and none when it has no budget. +func walkBudgetClass(p inventory.Plan, rollsOut func(string) bool) string { + if p.Release != nil || p.Times == nil { + return budgetNone + } + if _, budgeted := deliveryBudgets[p.Times.Class]; !budgeted { + return budgetNone + } + for _, tier := range p.Tiers { + for _, m := range tier { + if rollsOut(m) { + return p.Times.Class + } + } + } + return budgetNone +} + +// walkBound is a walk's bound by its budget class: twice its budget, or fifteen minutes with none. +func walkBound(class string) time.Duration { + if b, ok := deliveryBudgets[class]; ok { + return 2 * b + } + return walkNoBudgetBound +} + +// walkSends is what a walk will send at its start, per module the machines it goes to: each module of its tiers +// whose policy rolls it out, on every machine running it (for a release plan, those of its order). +func walkSends(p inventory.Plan, running func(string) []string, rollsOut func(string) bool) map[string][]string { + out := map[string][]string{} + for _, tier := range p.Tiers { + for _, m := range tier { + if s := p.Modules[m]; s != nil && s.State == planDeleted { + continue + } + if p.Release == nil && !rollsOut(m) { + continue + } + for _, n := range running(m) { + if p.Release != nil && !slices.Contains(p.Release.Order, n) { + continue + } + out[m] = appendNew(out[m], n) + } + } + } + return out +} + +// walkReached is what a walk sent by its end, per module the machines it reached: its first machines, its rest, +// and every module a gated send carried with it. A release plan reached the machines it walked: those done and +// the one being judged. +func walkReached(p inventory.Plan, running func(string) []string) map[string][]string { + out := map[string][]string{} + carried := func(g *inventory.PlanGate) { + if g == nil { + return + } + for _, c := range g.Carried { + out[c.Module] = appendNew(out[c.Module], c.Node) + } + } + if r := p.Release; r != nil { + reached := append([]string{}, r.Done...) + if r.Gate != nil { + reached = appendNew(reached, r.Gate.Machines...) + carried(r.Gate) + } + for _, tier := range p.Tiers { + for _, m := range tier { + for _, n := range running(m) { + if slices.Contains(reached, n) { + out[m] = appendNew(out[m], n) + } + } + } + } + return out + } + for m, s := range p.Modules { + if s == nil { + continue + } + out[m] = appendNew(out[m], s.First...) + for n := range s.Rest { + out[m] = appendNew(out[m], n) + } + carried(s.Gate) + if len(out[m]) == 0 { + delete(out, m) + } + } + return out +} + +// touchesOf turns what a walk moves, per module, into what it touches, per machine: the modules it moves there, +// and, where it moves the tool runner, every module there whose tools it serves, and where it moves the +// node-engine, every module there. Sorted by machine and module. +func touchesOf(moved map[string][]string, on map[string][]string, shelf map[string]catalogue.Manifest) []walkTouch { + byMachine := map[string][]string{} + for m, machines := range moved { + for _, n := range machines { + byMachine[n] = appendNew(byMachine[n], m) + } + } + out := make([]walkTouch, 0, len(byMachine)) + for n, modules := range byMachine { + t := walkTouch{Machine: n, Modules: modules} + engine, runner := slices.Contains(modules, hostModule), slices.Contains(modules, broker.RuntimeModule) + if engine || runner { + for _, m := range on[n] { + if slices.Contains(t.Modules, m) || (!engine && !servesTools(shelf[m])) { + continue + } + t.Modules = append(t.Modules, m) + t.Carried = append(t.Carried, m) + } + } + sort.Strings(t.Modules) + sort.Strings(t.Carried) + out = append(out, t) + } + sort.Slice(out, func(i, j int) bool { return out[i].Machine < out[j].Machine }) + return out +} + +// servesTools says the tool runner serves something of this module: a bundle it loads entrypoints from, or the +// module's own tools. +func servesTools(m catalogue.Manifest) bool { + if len(m.Tools) > 0 { + return true + } + for _, b := range m.Bundles { + if len(b.Loads) > 0 { + return true + } + } + return false +} + +// walkRead is the facts a walk's touches are worked out from, read once per event. +type walkRead struct { + running func(string) []string + rollsOut func(string) bool + onMachine func(moved map[string][]string) map[string][]string + shelf func() map[string]catalogue.Manifest +} + +// readWalkFacts reads what the walk's touches need. Without facts (a test, a command with no store), nothing runs +// anywhere and every module rolls out: the touches are then what the walk itself recorded. +func readWalkFacts(ctx context.Context, facts walkFacts, p inventory.Plan) walkRead { + cantRead := func(what string, err error) { + fmt.Fprintf(os.Stderr, "%s: %s cannot be read for what the walk touches: %v\n", p.ID, what, err) + } + r := walkRead{ + running: func(string) []string { return nil }, + rollsOut: func(string) bool { return true }, + onMachine: func(map[string][]string) map[string][]string { return nil }, + shelf: func() map[string]catalogue.Manifest { return nil }, + } + if facts == nil { + return r + } + running := map[string][]string{} + r.running = func(m string) []string { + if on, read := running[m]; read { + return on + } + on, err := facts.Running(ctx, m) + if err != nil { + cantRead("where "+m+" runs", err) + } + running[m] = on + return on + } + rolls := map[string]bool{} + r.rollsOut = func(m string) bool { + if yes, read := rolls[m]; read { + return yes + } + u, err := facts.UpgradeOf(ctx, m) + if err != nil { + cantRead(m+"'s policy", err) + } + // A policy that cannot be read is taken to roll out: the walk is then held more, never less. + rolls[m] = err != nil || u.RollOut + return rolls[m] + } + r.onMachine = func(moved map[string][]string) map[string][]string { + out := map[string][]string{} + for _, core := range []string{hostModule, broker.RuntimeModule} { + for _, n := range moved[core] { + if _, read := out[n]; read { + continue + } + var there []string + if sent, known, err := facts.SentBuilds(ctx, n); err != nil { + cantRead("what "+n+" was sent", err) + } else if known { + for m := range sent { + there = append(there, m) + } + } + if assigned, err := facts.Assigned(ctx, n); err != nil { + cantRead("what "+n+" is assigned", err) + } else { + there = appendNew(there, assigned...) + } + sort.Strings(there) + out[n] = there + } + } + return out + } + r.shelf = func() map[string]catalogue.Manifest { + shelf, err := facts.Catalogue(ctx) + if err != nil { + cantRead("the catalogue", err) + } + return shelf + } + return r +} diff --git a/cmd/mesh-controller/walk_said_test.go b/cmd/mesh-controller/walk_said_test.go new file mode 100644 index 00000000..70a4aaac --- /dev/null +++ b/cmd/mesh-controller/walk_said_test.go @@ -0,0 +1,306 @@ +package main + +import ( + "context" + "encoding/json" + "reflect" + "testing" + "time" + + "github.com/nats-io/nats.go" + + "github.com/novox/mesh-controller/internal/broker" + "github.com/novox/mesh-controller/internal/catalogue" + "github.com/novox/mesh-controller/internal/inventory" + "github.com/novox/mesh-controller/internal/link" + "github.com/novox/mesh-controller/internal/testbus" +) + +// fakeWalkFacts is the inventory as far as a walk's touches read it. +type fakeWalkFacts struct { + running map[string][]string + records map[string]bool // modules whose policy records rather than rolls out + sent map[string]map[string]string + assigned map[string][]string + shelf map[string]catalogue.Manifest +} + +func (f fakeWalkFacts) Running(_ context.Context, m string) ([]string, error) { + return f.running[m], nil +} +func (f fakeWalkFacts) UpgradeOf(_ context.Context, m string) (inventory.Upgrade, error) { + return inventory.Upgrade{RollOut: !f.records[m]}, nil +} +func (f fakeWalkFacts) Assigned(_ context.Context, n string) ([]string, error) { + return f.assigned[n], nil +} +func (f fakeWalkFacts) SentBuilds(_ context.Context, n string) (map[string]string, bool, error) { + s, ok := f.sent[n] + return s, ok, nil +} +func (f fakeWalkFacts) Catalogue(context.Context) (map[string]catalogue.Manifest, error) { + return f.shelf, nil +} + +// theMesh is four machines: the tool runner on each, messenger and mesh-issues on novox, gitea on ace. +func theMesh() fakeWalkFacts { + return fakeWalkFacts{ + running: map[string][]string{ + broker.RuntimeModule: {"ace", "g14", "novox", "shanks"}, + "messenger": {"novox"}, + "gitea": {"ace"}, + "plex": {"ace"}, + }, + sent: map[string]map[string]string{ + "novox": {broker.RuntimeModule: "c0", "messenger": "c1", "mesh-issues": "c2", "postgres": "c3"}, + "ace": {broker.RuntimeModule: "c0", "gitea": "c4"}, + }, + assigned: map[string][]string{"g14": {broker.RuntimeModule, "desktop"}, "shanks": {broker.RuntimeModule}}, + shelf: map[string]catalogue.Manifest{ + "messenger": {Module: "messenger", Tools: []string{"notify"}}, + "mesh-issues": {Module: "mesh-issues", Bundles: []catalogue.Bundle{{Name: "tools", Loads: []string{"main"}}}}, + "postgres": {Module: "postgres"}, + "gitea": {Module: "gitea", Tools: []string{"gitea_api"}}, + "desktop": {Module: "desktop", Tools: []string{"notify_send"}}, + }, + } +} + +func walkOfTiers(class string, tiers ...[]string) inventory.Plan { + cut := time.Date(2026, 10, 12, 11, 0, 0, 0, time.UTC) + started := cut.Add(time.Second) + p := inventory.Plan{ID: "plan-308", State: inventory.PlanBuilding, Tiers: tiers, + Modules: map[string]*inventory.PlanModule{}, Times: &inventory.PlanTimes{Cut: &cut, Class: class, Started: &started}} + for _, tier := range tiers { + for _, m := range tier { + p.Modules[m] = &inventory.PlanModule{} + } + } + p.WalkSaid = inventory.WalkStarted + return p +} + +// novox/hq ADR 0308 How it is checked §1: the start names the modules the walk moves and the machines it sends +// them to, and every module a moved tool runner carries there. +func TestAWalksStartNamesWhatItTouchesWithWhatTheToolRunnerCarries(t *testing.T) { + p := walkOfTiers(inventory.ClassCore, []string{broker.RuntimeModule}, []string{"messenger"}) + w := sayWalk(t.Context(), theMesh(), p) + if w == nil || w.Walk != "plan-308" || w.Phase != inventory.WalkStarted || w.Outcome != "" { + t.Fatalf("the start is said: %+v", w) + } + want := []walkTouch{ + {Machine: "ace", Modules: []string{"gitea", broker.RuntimeModule}, Carried: []string{"gitea"}}, + {Machine: "g14", Modules: []string{"desktop", broker.RuntimeModule}, Carried: []string{"desktop"}}, + // postgres serves no tools: the tool runner does not carry it. + {Machine: "novox", Modules: []string{"mesh-issues", "messenger", broker.RuntimeModule}, Carried: []string{"mesh-issues"}}, + {Machine: "shanks", Modules: []string{broker.RuntimeModule}}, + } + if !reflect.DeepEqual(w.Touches, want) { + t.Fatalf("touches:\n got %+v\nwant %+v", w.Touches, want) + } + if w.BudgetClass != inventory.ClassCore || w.BoundSeconds != 1200 || w.StartedAt == nil { + t.Fatalf("a core walk is bound at 20 minutes: %+v", w) + } +} + +// A moved node-engine carries every module on its machine; a module whose policy records is not sent, and not +// touched. +func TestAMovedNodeEngineCarriesEverythingOnItsMachine(t *testing.T) { + f := theMesh() + f.running[hostModule] = []string{"novox"} + f.records = map[string]bool{"plex": true} + p := walkOfTiers(inventory.ClassCore, []string{hostModule, "plex"}) + w := sayWalk(t.Context(), f, p) + want := []walkTouch{{Machine: "novox", + Modules: []string{"mesh-host", "mesh-issues", "messenger", "node-tools", "postgres"}, + Carried: []string{"mesh-issues", "messenger", "node-tools", "postgres"}}} + if !reflect.DeepEqual(w.Touches, want) { + t.Fatalf("touches:\n got %+v\nwant %+v", w.Touches, want) + } +} + +// The end names what the walk reached — its first machines, its rest, what a gated send carried — and its +// outcome; the modules a reached tool runner carries are touched there too. +func TestAWalksEndNamesWhatItReachedAndItsOutcome(t *testing.T) { + p := walkOfTiers(inventory.ClassCore, []string{broker.RuntimeModule}, []string{"messenger"}) + ended := p.Times.Started.Add(4 * time.Minute) + p.Times.Ended = &ended + p.State, p.WalkSaid = inventory.PlanDone, inventory.WalkEnded + p.Modules[broker.RuntimeModule] = &inventory.PlanModule{First: []string{"shanks"}, + Rest: map[string]inventory.SentDeclaration{"novox": {}}, + Gate: &inventory.PlanGate{Carried: []inventory.CarriedMove{{Module: "gitea", Node: "shanks"}}}} + // messenger was built and found unchanged: no move, not reached. + p.Modules["messenger"] = &inventory.PlanModule{Why: "no move"} + w := sayWalk(t.Context(), theMesh(), p) + if w.Phase != inventory.WalkEnded || w.Outcome != inventory.WalkDelivered || w.EndedAt == nil { + t.Fatalf("the end is said with its outcome: %+v", w) + } + want := []walkTouch{ + {Machine: "novox", Modules: []string{"mesh-issues", "messenger", broker.RuntimeModule}, + Carried: []string{"mesh-issues", "messenger"}}, + {Machine: "shanks", Modules: []string{"gitea", broker.RuntimeModule}}, + } + if !reflect.DeepEqual(w.Touches, want) { + t.Fatalf("touches:\n got %+v\nwant %+v", w.Touches, want) + } + for outcome, end := range map[string]func(*inventory.Plan){ + inventory.WalkFailed: func(p *inventory.Plan) { p.State = inventory.PlanFailed }, + inventory.WalkStopped: func(p *inventory.Plan) { + p.State, p.Delivery = inventory.PlanFailed, &inventory.PlanDelivery{Stopped: "a person"} + }, + } { + q := p + end(&q) + if got := sayWalk(t.Context(), theMesh(), q).Outcome; got != outcome { + t.Errorf("outcome %q, want %q", got, outcome) + } + } +} + +// novox/hq ADR 0308 How it is checked §1 and decision 5: the bound is twice the largest budget in force among +// the walk's deliveries — 20 minutes for a core walk, twice the leaf budget for a leaf walk — and 15 minutes, not +// doubled, for a walk none of whose deliveries has a budget. +func TestAWalksBoundIsTwiceItsBudgetOrFifteenMinutesWithNone(t *testing.T) { + f := theMesh() + f.records = map[string]bool{"plex": true} + release := walkOfTiers("", []string{"gitea"}) + release.Times, release.Release = &inventory.PlanTimes{}, &inventory.PlanRelease{Order: []string{"ace"}} + for name, c := range map[string]struct { + p inventory.Plan + class string + bound int + }{ + "core": {walkOfTiers(inventory.ClassCore, []string{"messenger"}), inventory.ClassCore, 1200}, + "leaf": {walkOfTiers(inventory.ClassLeaf, []string{"messenger"}), inventory.ClassLeaf, int(2 * deliveryBudgets[inventory.ClassLeaf] / time.Second)}, + "held for a person": {walkOfTiers(inventory.ClassLeaf, []string{"plex"}), budgetNone, 900}, + "a release": {release, budgetNone, 900}, + "kept without a class": {walkOfTiers("", []string{"messenger"}), budgetNone, 900}, + } { + w := sayWalk(t.Context(), f, c.p) + if w.BudgetClass != c.class || w.BoundSeconds != c.bound { + t.Errorf("%s: %s bound %ds, want %s %ds", name, w.BudgetClass, w.BoundSeconds, c.class, c.bound) + } + } + // The leaf budget in force is three minutes (ADR 0304 decision 4): a leaf walk is bound at six. + if deliveryBudgets[inventory.ClassLeaf] == 3*time.Minute { + if w := sayWalk(t.Context(), f, walkOfTiers(inventory.ClassLeaf, []string{"messenger"})); w.BoundSeconds != 360 { + t.Errorf("a leaf walk at a three-minute budget is bound at 360 s: %d", w.BoundSeconds) + } + } +} + +// plan-moved's body: the walk whole as before, and on its start the walk's fields beside it; on any other save +// none of them. +func TestPlanMovedCarriesTheWalkWholeAndOnItsStartWhatItTouches(t *testing.T) { + p := walkOfTiers(inventory.ClassLeaf, []string{"messenger"}) + body, err := json.Marshal(planMovedBody{Plan: p, walkSaid: sayWalk(t.Context(), theMesh(), p)}) + if err != nil { + t.Fatal(err) + } + var said map[string]any + if err := json.Unmarshal(body, &said); err != nil { + t.Fatal(err) + } + for _, k := range []string{"id", "state", "tiers", "modules", "times", "walk", "phase", "touches", "bound_seconds", "budget_class", "started_at"} { + if _, ok := said[k]; !ok { + t.Errorf("plan-moved lacks %q: %s", k, body) + } + } + if _, ok := said["outcome"]; ok { + t.Errorf("a start says no outcome: %s", body) + } + if said["walk"] != "plan-308" || said["phase"] != "started" || said["budget_class"] != "leaf" { + t.Errorf("the walk's fields: %s", body) + } + p.WalkSaid = "" + body, _ = json.Marshal(planMovedBody{Plan: p, walkSaid: sayWalk(t.Context(), theMesh(), p)}) + said = map[string]any{} + _ = json.Unmarshal(body, &said) + for _, k := range []string{"walk", "phase", "touches", "bound_seconds"} { + if _, ok := said[k]; ok { + t.Errorf("a save neither starting nor ending the walk says %q: %s", k, body) + } + } + // A walk that touches nothing says an empty list, never null. + q := walkOfTiers(inventory.ClassLeaf, []string{"nobody-runs-this"}) + body, _ = json.Marshal(planMovedBody{Plan: q, walkSaid: sayWalk(t.Context(), theMesh(), q)}) + said = map[string]any{} + _ = json.Unmarshal(body, &said) + if touches, ok := said["touches"].([]any); !ok || len(touches) != 0 { + t.Errorf("touches nothing: %s", body) + } +} + +// Live, with the store and a bus: a walk let go by the delivery's verb is said started with what it touches and +// its bound, and stopped by the verb is said ended, stopped (novox/hq ADR 0308 How it is checked §1). +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() }) + register(t, open, catalogue.Manifest{Module: "app", Version: "1"}) + if _, err := assign(ctx, open, "anchor", "app"); err != nil { + t.Fatal(err) + } + cut := time.Now().UTC() + waiting := inventory.Plan{ID: "plan-308-live", Repository: "novox/mesh-catalog", Branch: "main", Commit: "c8c8c8c8", + Created: cut, State: inventory.PlanBuilding, Tiers: [][]string{{"app"}}, + Modules: map[string]*inventory.PlanModule{"app": {}}, + Delivery: &inventory.PlanDelivery{Awaits: catalogue.DeliverySeat}, + Times: &inventory.PlanTimes{Cut: &cut, Class: inventory.ClassLeaf}} + 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) + } + started := next(inventory.WalkStarted) + touches, _ := json.Marshal(started["touches"]) + if started["walk"] != waiting.ID || string(touches) != `[{"machine":"anchor","modules":["app"]}]` || + started["budget_class"] != inventory.ClassLeaf || + started["bound_seconds"] != float64(2*deliveryBudgets[inventory.ClassLeaf]/time.Second) { + t.Fatalf("the start: %v", started) + } + if err := deliveryCommand(ctx, []string{"stop", waiting.ID, "--why", "a test", "--by", "a person"}); err != nil { + t.Fatal(err) + } + ended := next(inventory.WalkEnded) + touches, _ = json.Marshal(ended["touches"]) + if ended["walk"] != waiting.ID || ended["outcome"] != inventory.WalkStopped || string(touches) != `[]` { + t.Fatalf("the end, which sent nothing: %v", ended) + } +} diff --git a/internal/inventory/plans.go b/internal/inventory/plans.go index f90999a8..d044d29a 100644 --- a/internal/inventory/plans.go +++ b/internal/inventory/plans.go @@ -61,6 +61,10 @@ type Plan struct { // Phases is the walk's time from its merge, phase by phase (ADR 0282 decision 6): never kept, worked out // from the walk's own moments by whoever says the walk (`delivery walks`, `plan-moved`). Phases *WalkPhases `json:"phases,omitempty"` + // WalkSaid is, on the save that started or ended the walk, which of the two it was (novox/hq ADR 0308 + // decision 1): set by the save, never kept, read by whoever says the save (`plan-moved`). Empty on every + // other save. + WalkSaid string `json:"-"` } // PlanTimes are a walk's own moments beside its modules' (novox/hq ADR 0282 decision 6). @@ -73,6 +77,10 @@ type PlanTimes struct { // Class is the walk's module class, read at its cut: core when it moves a module on the controller's own // path or the mesh's resolver, leaf otherwise (ADR 0282 decision 1). Class string `json:"class,omitempty"` + // Started is when the walk was said started, and Ended when it was said ended (novox/hq ADR 0308 decision + // 1): each said once, on the save that marked it. + Started *time.Time `json:"started,omitempty"` + Ended *time.Time `json:"ended,omitempty"` } // PlanCommit is one repository's commit a walk carries: the latest merge of its branch in the batch. @@ -451,7 +459,7 @@ type MergeMove struct { // leaves a merge answered by two plans, nor a walk stopped whose merges were not moved). A plan written by // somebody else since it was read refuses the whole act with ErrPlanMoved. Each plan is said (PlanSaved) once // the transaction is committed, in order. -func (i *Inventory) SavePlansMoving(ctx context.Context, plans []*Plan, moves []MergeMove) error { +func (i *Inventory) SavePlansMoving(ctx context.Context, plans []*Plan, moves []MergeMove) (err error) { epoch, err := i.actingEpoch(ctx) if err != nil { if len(plans) > 0 { @@ -472,6 +480,20 @@ func (i *Inventory) SavePlansMoving(ctx context.Context, plans []*Plan, moves [] entered time.Time } done := make([]written, len(plans)) + // **The save that starts or ends a walk is marked** (novox/hq ADR 0308 decision 1), in the same write, and + // unmarked should the write not be made, so the next save marks it again. + marked := time.Now() + undo := make([]func(), 0, len(plans)) + defer func() { + if err != nil { + for _, u := range undo { + u() + } + } + }() + for _, p := range plans { + undo = append(undo, p.markWalk(marked)) + } for n, p := range plans { revision, entered, err := writePlan(ctx, tx, p, epoch) if err != nil { diff --git a/internal/inventory/walksaid.go b/internal/inventory/walksaid.go new file mode 100644 index 00000000..95c5749a --- /dev/null +++ b/internal/inventory/walksaid.go @@ -0,0 +1,81 @@ +package inventory + +import "time" + +// A walk says when it starts and when it ends (novox/hq ADR 0308 decision 1): the save that starts it and the +// save that ends it are marked, so `plan-moved` names what the walk touches, its bound and its outcome on those +// two saves, and on no other. The marks are kept in the walk's times, so a controller replaced mid-walk (a core +// walk replaces it) neither says a start again nor misses the end. + +// The moments of a walk `plan-moved` names. +const ( + WalkStarted = "started" + WalkEnded = "ended" +) + +// The outcomes of an ended walk (ADR 0308 decision 1). +const ( + WalkDelivered = "delivered" + WalkFailed = "failed" + WalkStopped = "stopped" +) + +// Live says the walk is walking: cut, not waiting for its delivery's word, and not ended. +func (p Plan) Live() bool { return p.Open() && !p.Batch() && !p.Waiting() } + +// Ended says the walk is in a state it does not leave by itself: done, failed or superseded. +func (p Plan) Ended() bool { + return p.State == PlanDone || p.State == PlanFailed || p.State == PlanSuperseded +} + +// Outcome is how an ended walk ended: delivered when done; stopped when a person or the delivery's owner stopped +// it, or a newer walk took it over; failed otherwise. Empty for a walk not ended. +func (p Plan) Outcome() string { + switch { + case p.State == PlanDone: + return WalkDelivered + case p.State == PlanSuperseded: + return WalkStopped + case p.State == PlanFailed && p.Delivery != nil && p.Delivery.Stopped != "": + return WalkStopped + case p.State == PlanFailed: + return WalkFailed + } + return "" +} + +// markWalk marks the save that starts or ends the walk, in its times and in WalkSaid, and returns what undoes +// the mark should the save not be written. A walk starts on the first save that finds it live — when it is cut +// and needs no word, or when it is let go — which is before its first tier is asked, and so before anything is +// sent: what it restarts is held from its first send on. It starts again when a failed walk is retried. It ends +// on the first save that finds it ended after it started; one that ends without ever starting (a batch that +// moved nothing, a walk stopped while it waited for its word) touched nothing and says neither. A walk kept +// before its times were (ADR 0282 decision 6) is not marked; a release plan is, its times holding only these. +func (p *Plan) markWalk(now time.Time) (undo func()) { + p.WalkSaid = "" + if p.Batch() || (p.Times == nil && p.Release == nil) { + return func() {} + } + var was *PlanTimes + if p.Times != nil { + kept := *p.Times + was = &kept + } + times := PlanTimes{} + if p.Times != nil { + times = *p.Times + } + at := now.UTC() + switch { + case p.Live() && (times.Started == nil || times.Ended != nil): + times.Started, times.Ended = &at, nil + p.WalkSaid = WalkStarted + case p.Ended() && times.Started != nil && times.Ended == nil: + times.Ended = &at + p.WalkSaid = WalkEnded + default: + return func() {} + } + p.Times = × + return func() { p.Times, p.WalkSaid = was, "" } +} diff --git a/internal/inventory/walksaid_test.go b/internal/inventory/walksaid_test.go new file mode 100644 index 00000000..98fa04b6 --- /dev/null +++ b/internal/inventory/walksaid_test.go @@ -0,0 +1,130 @@ +package inventory + +import ( + "testing" + "time" +) + +// A walk is said started on the save that finds it live, ended on the save that finds it ended after it started, +// and neither on any other save (novox/hq ADR 0308 decision 1). +func TestAWalkIsSaidStartedOnceAndEndedOnce(t *testing.T) { + now := time.Date(2026, 10, 12, 11, 0, 0, 0, time.UTC) + cut := now.Add(-time.Second) + p := Plan{ID: "plan-1", State: PlanBuilding, Tiers: [][]string{{"messenger"}}, + Modules: map[string]*PlanModule{"messenger": {}}, Times: &PlanTimes{Cut: &cut, Class: ClassLeaf}} + p.markWalk(now) + if p.WalkSaid != WalkStarted || p.Times.Started == nil || !p.Times.Started.Equal(now) { + t.Fatalf("a live walk's first save starts it: %q %+v", p.WalkSaid, p.Times) + } + p.State = PlanRolling + p.markWalk(now.Add(time.Minute)) + if p.WalkSaid != "" || !p.Times.Started.Equal(now) { + t.Fatalf("a later save says nothing and keeps the start: %q %+v", p.WalkSaid, p.Times) + } + p.State = PlanDone + p.markWalk(now.Add(2 * time.Minute)) + if p.WalkSaid != WalkEnded || p.Times.Ended == nil || p.Outcome() != WalkDelivered { + t.Fatalf("the save that finds it done ends it, delivered: %q %+v %q", p.WalkSaid, p.Times, p.Outcome()) + } + p.markWalk(now.Add(3 * time.Minute)) + if p.WalkSaid != "" { + t.Fatalf("an ended walk saved again says nothing: %q", p.WalkSaid) + } + // Retried, it walks again: a new start. + p.State = PlanBuilding + p.markWalk(now.Add(4 * time.Minute)) + if p.WalkSaid != WalkStarted || p.Times.Ended != nil || !p.Times.Started.Equal(now.Add(4*time.Minute)) { + t.Fatalf("a retried walk starts again: %q %+v", p.WalkSaid, p.Times) + } +} + +// What is never said: a batch, a walk waiting for its word, a walk that ends without having started, and a walk +// kept before its times were. A release plan is said. +func TestWhatIsNotAWalkStartIsNotSaid(t *testing.T) { + now := time.Now() + for name, p := range map[string]Plan{ + "a batch": {State: PlanAssembling, Times: &PlanTimes{}}, + "waiting for a word": {State: PlanBuilding, Times: &PlanTimes{}, Delivery: &PlanDelivery{Awaits: "mesh-delivery"}}, + "ended, never begun": {State: PlanDone, Times: &PlanTimes{}}, + "kept before times": {State: PlanBuilding}, + "stopped while waits": {State: PlanFailed, Times: &PlanTimes{}, Delivery: &PlanDelivery{Awaits: "mesh-delivery", Stopped: "x"}}, + } { + p.markWalk(now) + if p.WalkSaid != "" { + t.Errorf("%s: said %q", name, p.WalkSaid) + } + } + r := Plan{State: PlanRolling, Release: &PlanRelease{Order: []string{"ace"}}} + r.markWalk(now) + if r.WalkSaid != WalkStarted || r.Times == nil || r.Times.Started == nil || r.Times.Class != "" { + t.Fatalf("a release plan is said started, its times holding only that: %q %+v", r.WalkSaid, r.Times) + } + // Let go, the walk starts. + go1 := now + w := Plan{State: PlanBuilding, Times: &PlanTimes{}, Delivery: &PlanDelivery{Awaits: "mesh-delivery", Go: &go1}} + w.markWalk(now) + if w.WalkSaid != WalkStarted { + t.Fatalf("a walk let go starts: %q", w.WalkSaid) + } +} + +// An ended walk's outcome: delivered, failed, or stopped by a person or the delivery's owner, or by a newer walk. +func TestAWalksOutcome(t *testing.T) { + for want, p := range map[string]Plan{ + WalkDelivered: {State: PlanDone}, + WalkFailed: {State: PlanFailed}, + WalkStopped: {State: PlanFailed, Delivery: &PlanDelivery{Stopped: "mesh-delivery"}}, + "": {State: PlanRolling}, + } { + if got := p.Outcome(); got != want { + t.Errorf("%s: outcome %q, want %q", p.State, got, want) + } + } + if (Plan{State: PlanSuperseded}).Outcome() != WalkStopped { + t.Error("a superseded walk is stopped") + } +} + +// The marks are kept with the walk, so a controller replaced mid-walk neither says its start again nor misses +// its end; and a save refused leaves the walk unmarked, so the save that is made says it. +func TestAWalksStartIsKeptAndARefusedSaveLeavesItUnsaid(t *testing.T) { + inv := ForTest(t) + ctx := t.Context() + cut := time.Now().UTC().Add(-time.Minute) + p := Plan{ID: "plan-308", Repository: "novox/mesh-catalog", Commit: "abc", Created: cut, State: PlanBuilding, + Tiers: [][]string{{"messenger"}}, Modules: map[string]*PlanModule{"messenger": {}}, + Times: &PlanTimes{Cut: &cut, Class: ClassLeaf}} + var said []string + was := PlanSaved + PlanSaved = func(p Plan) { said = append(said, p.WalkSaid) } + t.Cleanup(func() { PlanSaved = was }) + if err := inv.SavePlan(ctx, &p); err != nil { + t.Fatal(err) + } + back, err := inv.PlanByID(ctx, p.ID) + if err != nil || back.Times == nil || back.Times.Started == nil || back.Times.Class != ClassLeaf { + t.Fatalf("the start is kept with the walk: %v %+v", err, back.Times) + } + // Read back by another controller and saved: not said again. + back.State = PlanRolling + if err := inv.SavePlan(ctx, &back); err != nil { + t.Fatal(err) + } + // A stale copy ending it is refused, and unmarked. + stale := p + stale.Revision = 1 + stale.State = PlanDone + if err := inv.SavePlan(ctx, &stale); err == nil { + t.Fatal("a stale save is refused") + } + if stale.WalkSaid != "" || stale.Times.Ended != nil { + t.Fatalf("a refused save is unmarked: %q %+v", stale.WalkSaid, stale.Times) + } + back.State = PlanDone + if err := inv.SavePlan(ctx, &back); err != nil { + t.Fatal(err) + } + if len(said) != 3 || said[0] != WalkStarted || said[1] != "" || said[2] != WalkEnded { + t.Fatalf("said: %q, want started, nothing, ended", said) + } +} diff --git a/testdata/store-tests b/testdata/store-tests index 98b101b2..729b6261 100644 --- a/testdata/store-tests +++ b/testdata/store-tests @@ -14,3 +14,5 @@ ./internal/inventory TestAnIssuedValueIsSealedAgainAfterAKeyChangeOrAnAcceptance ./internal/inventory TestAConditionsLooksAreKeptBesideTheBucket ./cmd/mesh-controller TestTheConditionsVerbReadsTheLooksTheBucketNoLongerHolds +./internal/inventory TestAWalksStartIsKeptAndARefusedSaveLeavesItUnsaid +./cmd/mesh-controller TestAWalkLetGoAndStoppedByVerbsIsSaidStartedAndEnded From cd6610cdf792bbe37f90883bd24bd166ca55b1b3 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 12 Oct 2026 12:08:34 +0200 Subject: [PATCH 2/3] 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 From 1e9dcf0667507d19d1bd1d08301d747664af447d Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 12 Oct 2026 12:29:18 +0200 Subject: [PATCH 3/3] Say every walk a verb keeps, and clear a stop from a walk that walks again cancel, clear and rebuild kept a walk's end or restart without saying it, as plans stop, retry and release-backlog did before them. The controller's run now says every save a verb makes on the bus, so the next verb cannot lose a mark the same way. A stop is cleared when a walk is resumed, so one that fails again ends failed and the search for its merge runs (review 21, ADR 0308, issue 532). --- cmd/mesh-controller/batches_test.go | 2 +- cmd/mesh-controller/delivery.go | 68 +++++++++++++- cmd/mesh-controller/main.go | 9 ++ cmd/mesh-controller/plan_retry.go | 12 +++ cmd/mesh-controller/push.go | 2 +- cmd/mesh-controller/walk_said_test.go | 123 ++++++++++++++++++++++++++ internal/inventory/plans.go | 12 +-- internal/inventory/walksaid_test.go | 2 +- testdata/store-tests | 3 + 9 files changed, 223 insertions(+), 10 deletions(-) diff --git a/cmd/mesh-controller/batches_test.go b/cmd/mesh-controller/batches_test.go index 846804ca..23557cfb 100644 --- a/cmd/mesh-controller/batches_test.go +++ b/cmd/mesh-controller/batches_test.go @@ -226,7 +226,7 @@ func TestTheAssemblingBatchIsShown(t *testing.T) { asksWithPaths(t) var said []inventory.Plan was := inventory.PlanSaved - inventory.PlanSaved = func(p inventory.Plan) { said = append(said, p) } + inventory.PlanSaved = func(_ *inventory.Inventory, p inventory.Plan) { said = append(said, p) } t.Cleanup(func() { inventory.PlanSaved = was }) now := time.Now().UTC() claude := catalogueMerge("553b7191claude", "app", now.Add(-20*time.Second)) diff --git a/cmd/mesh-controller/delivery.go b/cmd/mesh-controller/delivery.go index 23a0a629..c5f4765f 100644 --- a/cmd/mesh-controller/delivery.go +++ b/cmd/mesh-controller/delivery.go @@ -10,6 +10,7 @@ import ( "slices" "sort" "strings" + "sync" "time" "github.com/nats-io/nats.go" @@ -1253,9 +1254,10 @@ func sayingOnTheBus(ctx context.Context, facts walkFacts, f func() error) error return err } bus := link.OverNATS{Conn: conn, JS: js} + was := inventory.PlanSaved checkEvents = bus - inventory.PlanSaved = func(p inventory.Plan) { publishPlanMoved(ctx, bus, facts, p) } - defer func() { checkEvents, inventory.PlanSaved = nil, nil }() + inventory.PlanSaved = func(_ *inventory.Inventory, p inventory.Plan) { publishPlanMoved(ctx, bus, facts, p) } + defer func() { checkEvents, inventory.PlanSaved = nil, was }() ran = true return f() }) @@ -1265,3 +1267,65 @@ func sayingOnTheBus(ctx context.Context, facts walkFacts, f func() error) error } return err } + +// sayingWhatAVerbKept is PlanSaved for a process that is not the serving controller: each walk the verb keeps is +// said as `plan-moved` before the save returns, with the facts of the inventory that kept it, on the bus the verb +// reaches. It reaches the bus on the first save and never for a verb that keeps no walk. When the bus cannot be +// reached, a save that started or ended a walk says so on the verb's error output: its mark is kept and no later +// save says it, so the router holds what the walk touched until its bound, or not at all for a start. done lets +// the bus go and takes the hook back. +func sayingWhatAVerbKept(ctx context.Context) (said func(*inventory.Inventory, inventory.Plan), done func()) { + var ( + mu sync.Mutex + reached bool + bus link.Bus + closing func() + failed error + ) + said = func(inv *inventory.Inventory, p inventory.Plan) { + mu.Lock() + defer mu.Unlock() + if !reached { + reached = true + if handActConn != nil { + bus, failed = overNATS(handActConn) + } else if js, err := aBus(); err != nil { + failed = err + } else if bus, failed = overNATS(js.Conn()); failed == nil { + closing = js.Close + } else { + js.Close() + } + } + if bus == nil { + switch p.WalkSaid { + case inventory.WalkStarted: + fmt.Fprintf(os.Stderr, "%s: its start is kept and could NOT be said as plan-moved (%v): the router "+ + "does not hold what it touches\n", p.ID, failed) + case inventory.WalkEnded: + fmt.Fprintf(os.Stderr, "%s: its end is kept and could NOT be said as plan-moved (%v): the router holds "+ + "what it touched until its bound\n", p.ID, failed) + } + return + } + publishPlanMoved(ctx, bus, inv, p) + } + done = func() { + mu.Lock() + defer mu.Unlock() + inventory.PlanSaved = nil + if closing != nil { + closing() + } + } + return said, done +} + +// overNATS is the bus a connection carries, with its JetStream. +func overNATS(conn *nats.Conn) (link.Bus, error) { + js, err := conn.JetStream() + if err != nil { + return nil, err + } + return link.OverNATS{Conn: conn, JS: js}, nil +} diff --git a/cmd/mesh-controller/main.go b/cmd/mesh-controller/main.go index 3e4cebec..7269d761 100644 --- a/cmd/mesh-controller/main.go +++ b/cmd/mesh-controller/main.go @@ -63,6 +63,15 @@ func run() error { // 45 §6), so the next controller takes it at once rather than after its age. defer theLease.release() + // Every walk a verb keeps is said on the bus, whichever verb kept it (novox/hq ADR 0308 decision 1): the save + // that starts or ends a walk keeps its mark, and no later save says it again. The serving controller says its + // own saves. + if args[0] != "serve" { + said, done := sayingWhatAVerbKept(ctx) + inventory.PlanSaved = said + defer done() + } + switch args[0] { case "build": return buildCommand(ctx, args[1:]) diff --git a/cmd/mesh-controller/plan_retry.go b/cmd/mesh-controller/plan_retry.go index a41acb69..93a3bcdc 100644 --- a/cmd/mesh-controller/plan_retry.go +++ b/cmd/mesh-controller/plan_retry.go @@ -251,6 +251,16 @@ func resumed(p *inventory.Plan, why string) { if p.State == inventory.PlanFailed && len(failedIn(*p)) == 0 { p.State = inventory.PlanBuilding p.Note = why + goesOnAgain(p) + } +} + +// goesOnAgain clears a stop from a walk that walks again: a stop said why the walk ended, and a walk resumed has +// not ended. Kept, it would end a retried walk that fails again as stopped, not failed (novox/hq ADR 0308 +// decision 1), and keep the search for the merge that brought the failure from running (searchAfterFailures). +func goesOnAgain(p *inventory.Plan) { + if p.Delivery != nil { + p.Delivery.Stopped, p.Delivery.StoppedWhy = "", "" } } @@ -460,6 +470,7 @@ func retryRollouts(ctx context.Context, open *stores, p *inventory.Plan) (string said = append(said, m+" to "+strings.Join(again, ", ")) } p.State = inventory.PlanRolling + goesOnAgain(p) p.Note = fmt.Sprintf("tier %d retried by hand; sent %s first again", p.Tier, strings.Join(said, "; ")) if err := open.inventory.SavePlan(ctx, p); err != nil { return "", err @@ -525,6 +536,7 @@ func retryTierWhole(ctx context.Context, open *stores, p *inventory.Plan, again } if p.State == inventory.PlanFailed && len(failedIn(*p)) == 0 { p.State = inventory.PlanBuilding + goesOnAgain(p) p.Note = fmt.Sprintf("tier %d retried by hand: %s asked again; %s sent again with the tier", p.Tier, strings.Join(again, ", "), orNone(strings.Join(resent, ", "))) } diff --git a/cmd/mesh-controller/push.go b/cmd/mesh-controller/push.go index f550fe03..723eb7bc 100644 --- a/cmd/mesh-controller/push.go +++ b/cmd/mesh-controller/push.go @@ -249,7 +249,7 @@ func serve(ctx context.Context) (err error) { // (novox/hq to-be 45 §9). checkEvents = bus // And every walk this controller keeps in a new state, for the delivery it walks (novox/hq ADR 0239). - inventory.PlanSaved = func(p inventory.Plan) { sayPlanMoved(ctx, bus, open.inventory, p) } + inventory.PlanSaved = func(_ *inventory.Inventory, p inventory.Plan) { sayPlanMoved(ctx, bus, open.inventory, p) } if err := server.Checks(following{open}); err != nil { return err } diff --git a/cmd/mesh-controller/walk_said_test.go b/cmd/mesh-controller/walk_said_test.go index a5075508..22ef9a15 100644 --- a/cmd/mesh-controller/walk_said_test.go +++ b/cmd/mesh-controller/walk_said_test.go @@ -3,6 +3,7 @@ package main import ( "context" "encoding/json" + "os" "reflect" "testing" "time" @@ -389,3 +390,125 @@ func TestABacklogReleasedByAPersonIsSaidStarted(t *testing.T) { t.Fatalf("the release's start: %v", started) } } + +// asTyped runs a verb as the controller's command line runs it, through run, as a person's terminal would. +func asTyped(t *testing.T, args ...string) error { + t.Helper() + was := os.Args + os.Args = append([]string{"mesh-controller"}, args...) + t.Cleanup(func() { os.Args = was }) + err := run() + // run gives back the lease as the process would end; the test's process goes on, and takes it again. + theLease.mu.Lock() + theLease.held, theLease.kv, theLease.noBus = nil, nil, false + theLease.mu.Unlock() + return err +} + +// aStartedWalkAsking keeps a live walk, said started, whose first tier asked a as build. +func aStartedWalkAsking(t *testing.T, open *stores, id, build string, asked time.Time) inventory.Plan { + t.Helper() + walk := inventory.Plan{ID: id, Repository: "novox/a", Branch: "main", Commit: "c0ffee", Created: asked, + State: inventory.PlanBuilding, Tiers: [][]string{{"a"}, {"b"}}, + Modules: map[string]*inventory.PlanModule{"a": {State: "asked", AskedAt: &asked, Build: build}}, + Times: &inventory.PlanTimes{Cut: &asked, Started: &asked, Class: inventory.ClassLeaf}} + if err := open.inventory.SavePlan(t.Context(), &walk); err != nil { + t.Fatal(err) + } + return walk +} + +// Live, with the store and a bus: `cancel ` and `clear` fail the walk that asked the build, and the verb +// says its end, failed (novox/hq ADR 0308 decision 1, review 21 on mesh-controller#247). +func TestAWalkEndedByCancelOrClearIsSaidEnded(t *testing.T) { + _, ask := aBuildQueue(t) + open := aMesh(t) + next := planMovedHeard(t) + twoTiers(t, open) + for _, verb := range [][]string{{"cancel"}, {"clear"}} { + id := link.NewBuildID(time.Now()) + ask(id, "https://forge.example/novox/a.git") + asked, _ := link.BuildAskedAt(id) + walk := aStartedWalkAsking(t, open, "plan-308-"+verb[0], id, asked) + args := verb + if verb[0] == "cancel" { + args = append(args, id) + } + if err := asTyped(t, args...); err != nil { + t.Fatal(err) + } + ended := next(inventory.WalkEnded) + if ended["walk"] != walk.ID || ended["outcome"] != inventory.WalkFailed { + t.Fatalf("%s: the end is %v", verb[0], ended) + } + // Ended, and so the next walk may be cut on the same repository. + p, _ := open.inventory.PlanByID(t.Context(), walk.ID) + if p.State != inventory.PlanFailed { + t.Fatalf("%s left the walk %s", verb[0], p.State) + } + } +} + +// Live, with the store and a bus: `rebuild ` joining a failed walk sets it building again, and the verb +// says its start (novox/hq ADR 0308 decision 1, review 21 on mesh-controller#247). +func TestAWalkRestartedByARebuildIsSaidStarted(t *testing.T) { + open := aMesh(t) + 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-rebuild", Repository: "novox/a", Branch: "main", Commit: "c0ffee", + Created: before, State: inventory.PlanFailed, Tiers: [][]string{{"a"}, {"b"}}, + Modules: map[string]*inventory.PlanModule{"a": {State: "failed", AskedAt: &before, Build: "build-1", + Why: link.CancelledByHand}}, + Times: &inventory.PlanTimes{Cut: &before, Started: &before, Ended: &ended, Class: inventory.ClassLeaf}} + if err := open.inventory.SavePlan(t.Context(), &failed); err != nil { + t.Fatal(err) + } + if err := asTyped(t, "rebuild", "a"); err != nil { + t.Fatal(err) + } + if started := next(inventory.WalkStarted); started["walk"] != failed.ID { + t.Fatalf("the rebuilt walk's start: %v", started) + } +} + +// Live, with the store: a walk a person stopped and then retried, by `plans retry` or by `rebuild`, walks again +// without the stop, so when it fails again it ends failed, not stopped, and the search for the merge that +// brought the failure is not kept from it (novox/hq ADR 0308 decision 1, review 21 on mesh-controller#247). +func TestAWalkStoppedByHandAndRetriedEndsFailedWhenItFailsAgain(t *testing.T) { + open := aMesh(t) + ctx := t.Context() + asksRecorded(t) + twoTiers(t, open) + for _, again := range [][]string{{"plans", "retry"}, {"rebuild", "a"}} { + before := time.Now().UTC().Add(-time.Hour) + walk := aStartedWalkAsking(t, open, "plan-308-again-"+again[0], "build-1", before) + if err := asTyped(t, "plans", "stop", walk.ID, "--why", "a test"); err != nil { + t.Fatal(err) + } + // The build asked before the stop fails after it. + p, _ := open.inventory.PlanByID(ctx, walk.ID) + p.Modules["a"].State, p.Modules["a"].Why = "failed", "it broke" + if err := open.inventory.SavePlan(ctx, &p); err != nil { + t.Fatal(err) + } + args := again + if again[0] == "plans" { + args = append(append([]string(nil), again...), walk.ID) + } + if err := asTyped(t, args...); err != nil { + t.Fatal(err) + } + p, _ = open.inventory.PlanByID(ctx, walk.ID) + if p.State != inventory.PlanBuilding || (p.Delivery != nil && p.Delivery.Stopped != "") { + t.Fatalf("%s: the walk is %s and keeps its stop: %+v", again[0], p.State, p.Delivery) + } + asking, _ := link.BuildAskedAt(p.Modules["a"].Build) + planBuilt(ctx, open, "a", "c0ffee", "it broke again", asking, p.Modules["a"].Build) + if p, _ = open.inventory.PlanByID(ctx, walk.ID); p.State != inventory.PlanFailed || p.Outcome() != inventory.WalkFailed { + t.Fatalf("%s: failing again, the walk is %s and ends %s", again[0], p.State, p.Outcome()) + } + } +} diff --git a/internal/inventory/plans.go b/internal/inventory/plans.go index a0ec1bf8..fa7f3231 100644 --- a/internal/inventory/plans.go +++ b/internal/inventory/plans.go @@ -244,10 +244,12 @@ func (p Plan) Waiting() bool { return p.Delivery != nil && p.Delivery.Awaits != "" && p.Delivery.Go == nil } -// PlanSaved is told every plan this process kept, after it is kept (novox/hq ADR 0239): the serving -// controller says it on the bus as `plan-moved`. Nil in a command, which says nothing; whoever follows a -// walk also asks for it, so a save made by a command is found by comparison. -var PlanSaved func(Plan) +// PlanSaved is told every plan this process kept, after it is kept, with the inventory that kept it (novox/hq +// ADR 0239): the serving controller says it on the bus as `plan-moved`, and so does a command (the controller's +// run installs it for every verb, novox/hq ADR 0308 decision 1), so the save that starts or ends a walk is said +// whichever process made it. Nil says nothing; whoever follows a walk also asks for it, and finds an unsaid save +// by comparison. +var PlanSaved func(*Inventory, Plan) // ErrPlanMoved is a save against a plan written by somebody else since it was read. var ErrPlanMoved = errors.New("the plan was written by somebody else since it was read") @@ -519,7 +521,7 @@ func (i *Inventory) SavePlansMoving(ctx context.Context, plans []*Plan, moves [] p.Epoch = 0 } if PlanSaved != nil { - PlanSaved(*p) + PlanSaved(i, *p) } } return nil diff --git a/internal/inventory/walksaid_test.go b/internal/inventory/walksaid_test.go index 98fa04b6..69cf2e5e 100644 --- a/internal/inventory/walksaid_test.go +++ b/internal/inventory/walksaid_test.go @@ -96,7 +96,7 @@ func TestAWalksStartIsKeptAndARefusedSaveLeavesItUnsaid(t *testing.T) { Times: &PlanTimes{Cut: &cut, Class: ClassLeaf}} var said []string was := PlanSaved - PlanSaved = func(p Plan) { said = append(said, p.WalkSaid) } + PlanSaved = func(_ *Inventory, p Plan) { said = append(said, p.WalkSaid) } t.Cleanup(func() { PlanSaved = was }) if err := inv.SavePlan(ctx, &p); err != nil { t.Fatal(err) diff --git a/testdata/store-tests b/testdata/store-tests index 8b270fb1..d60bca8d 100644 --- a/testdata/store-tests +++ b/testdata/store-tests @@ -19,3 +19,6 @@ ./cmd/mesh-controller TestAWalkStoppedOrClosedByAPersonIsSaidEndedStopped ./cmd/mesh-controller TestAWalkRetriedByAPersonIsSaidStartedAgain ./cmd/mesh-controller TestABacklogReleasedByAPersonIsSaidStarted +./cmd/mesh-controller TestAWalkEndedByCancelOrClearIsSaidEnded +./cmd/mesh-controller TestAWalkRestartedByARebuildIsSaidStarted +./cmd/mesh-controller TestAWalkStoppedByHandAndRetriedEndsFailedWhenItFailsAgain