From fbfdded74beafd63fd0564d7a8c8f21d710e00c2 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 11 Oct 2026 19:54:12 +0200 Subject: [PATCH 1/5] Let delivery-stop leave merges out and walk the others again, so a group stop ends only its own change (hq issue 459, ADR 0299) One walk carries every merge of its batch, so ending it for one group member ended every other delivery on it. delivery-stop now takes without: the walk ends, every other merge it answered is batched again, and each later merge of a left-out merge's repository is left out too, since it contains it. --- cmd/mesh-controller/delivery.go | 176 ++++++++++++++++++++++++-- cmd/mesh-controller/issue459_test.go | 177 +++++++++++++++++++++++++++ cmd/mesh-controller/seatverbs.go | 4 + internal/catalogue/verbs.go | 8 +- internal/inventory/plans.go | 15 +++ 5 files changed, 370 insertions(+), 10 deletions(-) create mode 100644 cmd/mesh-controller/issue459_test.go diff --git a/cmd/mesh-controller/delivery.go b/cmd/mesh-controller/delivery.go index 0a8a4a88..1a4d7400 100644 --- a/cmd/mesh-controller/delivery.go +++ b/cmd/mesh-controller/delivery.go @@ -117,28 +117,180 @@ func letGo(ctx context.Context, inv *inventory.Inventory, id, by, why string) (i // stopWalk ends a walk on its delivery's word: failed, said as stopped by whom, why. What it asked still // builds and registers; nothing further is asked or sent. func stopWalk(ctx context.Context, inv *inventory.Inventory, id, by, why string) (inventory.Plan, error) { + p, _, err := stopWalkWithout(ctx, inv, id, by, why, nil, time.Now().UTC()) + return p, err +} + +// stopAnswer is what a stop that leaves merges out says, as data (novox/hq ADR 0299): the walk stopped, the +// batch that walks its other merges again, those merges as `@`, and every merge left out. +type stopAnswer struct { + Stopped string `json:"stopped"` + WalkedAgainBy string `json:"walked_again_by,omitempty"` + WalkedAgain []string `json:"walked_again,omitempty"` + LeftOut []inventory.PlanLeftOut `json:"left_out,omitempty"` +} + +// stopWalkWithout ends a walk on its delivery's word and, given merges to leave out, walks every other merge it +// answered again in a fresh batch without them (novox/hq issue 459, ADR 0299): one walk carries every merge of its +// batch, so ending it for one group member ended every other delivery on it. Left out are the merges named and +// each later merge of their repository's branch, which contains one of them and cannot be walked without +// delivering it. The fresh batch's window is closed — its merges were heard before — and it is cut by the usual +// rules, as a failed walk's merges are walked again (ADR 0276 decision 3). With nothing left to walk, or no merge +// named, the walk is only ended. A merge named that the walk does not answer refuses the stop whole. +func stopWalkWithout(ctx context.Context, inv *inventory.Inventory, id, by, why string, without []string, + now time.Time) (inventory.Plan, stopAnswer, error) { + said := stopAnswer{Stopped: id} release, err := inv.HoldPlans(ctx, true) if err != nil { - return inventory.Plan{}, err + return inventory.Plan{}, said, err } defer release() p, err := inv.PlanByID(ctx, id) if err != nil { - return inventory.Plan{}, err + return inventory.Plan{}, said, err } if !p.Open() { - return p, fmt.Errorf("%s is already %s", p.ID, p.State) + return p, said, fmt.Errorf("%s is already %s", p.ID, p.State) + } + var again []inventory.BatchedMerge + if len(without) > 0 { + merges, err := inv.MergesOf(ctx, p.ID) + if err != nil { + return p, said, err + } + if again, said.LeftOut, err = leaveOut(merges, without); err != nil { + return p, said, fmt.Errorf("%s is not stopped: %w", p.ID, err) + } } if p.Delivery == nil { p.Delivery = &inventory.PlanDelivery{} } p.Delivery.Stopped, p.Delivery.StoppedWhy = by, why + p.Delivery.LeftOut = said.LeftOut p.State = inventory.PlanFailed p.Note = fmt.Sprintf("stopped by %s at tier %d: %s", by, p.Tier, why) - if err := inv.SavePlan(ctx, &p); err != nil { - return p, err + if len(again) > 0 { + batch, err := walkAgain(ctx, inv, p, again, now) + if err != nil { + return p, said, fmt.Errorf("%s is not stopped: its other merges could not be batched again: %w", p.ID, err) + } + said.WalkedAgainBy = batch.ID + for _, m := range again { + said.WalkedAgain = append(said.WalkedAgain, m.Repository+"@"+m.Commit) + } + p.Delivery.WalkedAgainBy = batch.ID + p.Note += fmt.Sprintf("; its other merges are walked again without %s, by %s: %s", mergesLeftOutWords(said.LeftOut), + batch.ID, strings.Join(said.WalkedAgain, ", ")) + if err := inv.SavePlan(ctx, &p); err != nil { + // The merges go back to the walk that still answers them, and the batch holds none: it is never walked. + for _, m := range again { + _ = inv.AnswerMerge(ctx, m.Repository, m.Commit, p.ID, m.Alone) + } + return p, said, err + } + fmt.Printf("%s stopped by %s; %s walks its other merges again: %s\n", p.ID, by, batch.ID, + strings.Join(said.WalkedAgain, ", ")) + return p, said, nil } - return p, nil + if len(said.LeftOut) > 0 { + p.Note += fmt.Sprintf("; left out %s, nothing else to walk", mergesLeftOutWords(said.LeftOut)) + } + if err := inv.SavePlan(ctx, &p); err != nil { + return p, said, err + } + return p, said, nil +} + +// walkAgain puts merges of a stopped walk into a fresh batch of their kind, its window closed: kept before its +// first merge names it, then each merge moved to it. +func walkAgain(ctx context.Context, inv *inventory.Inventory, stopped inventory.Plan, again []inventory.BatchedMerge, + now time.Time) (inventory.Plan, error) { + own := false + for _, m := range again { + for _, n := range announcedOf(m).Moves { + if _, on := onTheControllersPath[n]; on { + own = true + } + } + } + first := again[0] + b := inventory.Plan{ID: fmt.Sprintf("plan-%d", now.UnixNano()), Repository: first.Repository, Branch: first.Branch, + Commit: first.Commit, Merged: first.Merged, Created: now, State: inventory.PlanAssembling, + Tiers: [][]string{}, Modules: map[string]*inventory.PlanModule{}, + Delivery: &inventory.PlanDelivery{Batch: &inventory.PlanBatch{Own: own}}} + if b.ID == stopped.ID { + b.ID += "-again" + } + if err := inv.SavePlan(ctx, &b); err != nil { + return b, err + } + for _, m := range again { + if err := inv.AnswerMerge(ctx, m.Repository, m.Commit, b.ID, m.Alone); err != nil { + return b, err + } + } + // Its merges were heard before: the window, measured from the last of them, is closed, and the cutter cuts it + // once no walk of its kind is open. + return b, keepBatch(ctx, inv, &b, now, "") +} + +// leaveOut splits a walk's merges by the merges named to leave out, each `@` (a commit's +// first characters will do): the merges walked again, and those left out — each named one, and each later merge +// of its repository's branch, which contains it. A name the walk does not answer is refused. +func leaveOut(merges []inventory.BatchedMerge, without []string) ([]inventory.BatchedMerge, []inventory.PlanLeftOut, error) { + var stopped []inventory.BatchedMerge + for _, w := range without { + repository, commit, ok := strings.Cut(strings.TrimSpace(w), "@") + if !ok || repository == "" || len(commit) < 7 { + return nil, nil, fmt.Errorf("%q names no merge: @", w) + } + i := slices.IndexFunc(merges, func(m inventory.BatchedMerge) bool { + return strings.EqualFold(m.Repository, repository) && strings.HasPrefix(m.Commit, commit) + }) + if i < 0 { + return nil, nil, fmt.Errorf("it answers no merge %s@%s", repository, commit) + } + stopped = append(stopped, merges[i]) + } + // The earliest stopped merge of each repository's branch: every merge of that branch after it contains it. + earliest := map[string]inventory.BatchedMerge{} + for _, s := range stopped { + k := strings.ToLower(s.Repository) + "\x00" + s.Branch + if e, ok := earliest[k]; !ok || laterMerge(e, s) { + earliest[k] = s + } + } + var again []inventory.BatchedMerge + var out []inventory.PlanLeftOut + for _, m := range merges { + named := slices.ContainsFunc(stopped, func(s inventory.BatchedMerge) bool { + return strings.EqualFold(s.Repository, m.Repository) && s.Commit == m.Commit + }) + e, has := earliest[strings.ToLower(m.Repository)+"\x00"+m.Branch] + switch { + case named: + out = append(out, inventory.PlanLeftOut{Repository: m.Repository, Commit: m.Commit}) + case has && laterMerge(m, e): + out = append(out, inventory.PlanLeftOut{Repository: m.Repository, Commit: m.Commit, Contains: e.Commit}) + default: + again = append(again, m) + } + } + sort.SliceStable(out, func(i, j int) bool { return out[i].Contains == "" && out[j].Contains != "" }) + return again, out, nil +} + +// mergesLeftOutWords is the merges a stop left out, as a person reads them: each that contains a stopped one says so. +func mergesLeftOutWords(out []inventory.PlanLeftOut) string { + var words []string + for _, o := range out { + s := o.Repository + "@" + short(o.Commit) + if o.Contains != "" { + s += " (which contains " + short(o.Contains) + ")" + } + words = append(words, s) + } + return strings.Join(words, ", ") } // orderMember is one member of a group, as mesh-delivery says it. @@ -466,6 +618,8 @@ func deliveryCommand(ctx context.Context, args []string) error { group := set.String("group", "", "a delivery group's id") by := set.String("by", catalogue.DeliverySeat, "who says it") why := set.String("why", "", "why") + without := set.String("without", "", "merges a stop leaves out, @, comma-separated: the walk's "+ + "other merges are walked again without them (novox/hq ADR 0299)") limit := set.Int("n", 50, "how many ended walks to answer beside the open ones") planID := set.String("plan", "", "one walk") positionals, err := parseAround(set, rest) @@ -552,15 +706,21 @@ func deliveryCommand(ctx context.Context, args []string) error { return nil case "stop": if len(positionals) != 1 || strings.TrimSpace(*why) == "" { - return errors.New("delivery stop --why [--by ]") + return errors.New("delivery stop --why [--by ] [--without @,…]") } var p inventory.Plan + var said stopAnswer if err := sayingOnTheBus(ctx, func() (err error) { - p, err = stopWalk(ctx, inv, positionals[0], *by, strings.TrimSpace(*why)) + p, said, err = stopWalkWithout(ctx, inv, positionals[0], *by, strings.TrimSpace(*why), splitList(*without), + time.Now().UTC()) return err }); err != nil { return err } + if *without != "" { + // What it left out and what it walks again, as data: the delivery's owner reads it (ADR 0299). + return answer(said) + } fmt.Printf("%s stopped by %s at tier %d of %d; what was asked still builds and registers, nothing further "+ "is asked or sent\n", p.ID, *by, p.Tier, len(p.Tiers)) return nil diff --git a/cmd/mesh-controller/issue459_test.go b/cmd/mesh-controller/issue459_test.go new file mode 100644 index 00000000..c5591b3d --- /dev/null +++ b/cmd/mesh-controller/issue459_test.go @@ -0,0 +1,177 @@ +package main + +import ( + "reflect" + "strings" + "testing" + "time" + + "github.com/novox/mesh-controller/internal/inventory" +) + +// novox/hq issue 459, ADR 0299: since the merge window one walk carries every merge of its batch, so a stop of one +// group member through `delivery-stop` ended the walk of every other delivery in it. The stop now names the merges +// to leave out: the walk is ended, and every other merge it answered is walked again in a fresh batch without them +// — the stopped merge, and each later merge of its repository's branch, which contains it and ends with it. + +// leaved is a kept merge of a repository, made at a moment. +func leaved(repository, commit string, made time.Time) inventory.BatchedMerge { + return inventory.BatchedMerge{Repository: repository, Branch: "main", Commit: commit, Merged: made, Heard: made} +} + +// The stopped merge is left out, and so is each later merge of its repository's branch, naming the merge it +// contains; an earlier merge of that repository and every other repository's merge are walked again. +func TestLeftOutAreTheStoppedMergeAndTheMergesThatContainIt(t *testing.T) { + c1 := leaved("novox/mesh-catalog", "c1aaaaaaaaaa", t0) + c2 := leaved("novox/mesh-catalog", "c2bbbbbbbbbb", t0.Add(10*time.Second)) + c3 := leaved("novox/mesh-catalog", "c3cccccccccc", t0.Add(20*time.Second)) + o1 := leaved("novox/one", "o1dddddddddd", t0.Add(5*time.Second)) + again, out, err := leaveOut([]inventory.BatchedMerge{c1, o1, c2, c3}, []string{"novox/Mesh-Catalog@c2bbbbbbbbbb"}) + if err != nil { + t.Fatal(err) + } + var walked []string + for _, m := range again { + walked = append(walked, m.Commit) + } + if !reflect.DeepEqual(walked, []string{c1.Commit, o1.Commit}) { + t.Fatalf("walked again: %v; want the earlier catalogue merge and the other repository's", walked) + } + want := []inventory.PlanLeftOut{{Repository: "novox/mesh-catalog", Commit: c2.Commit}, + {Repository: "novox/mesh-catalog", Commit: c3.Commit, Contains: c2.Commit}} + if !reflect.DeepEqual(out, want) { + t.Fatalf("left out: %+v; want %+v", out, want) + } + // A merge the walk does not answer is refused, naming it: nothing ends. + if _, _, err := leaveOut([]inventory.BatchedMerge{c1, o1}, []string{"novox/mesh-catalog@ffffffffffff"}); err == nil || + !strings.Contains(err.Error(), "ffffffffffff") { + t.Fatalf("a merge the walk does not answer was left out: %v", err) + } + if _, _, err := leaveOut([]inventory.BatchedMerge{c1}, []string{"no-commit"}); err == nil { + t.Fatal("a merge without its commit was taken") + } +} + +// The seat verb passes the merges to leave out to the command. +func TestDeliveryStopTakesTheMergesToLeaveOut(t *testing.T) { + got, err := argvFor("delivery-stop", map[string]any{"plan": "plan-1", "why": "w", "by": "jochen", + "without": "novox/mesh-lab@b1,novox/mesh-lab@b2"}) + want := []string{"delivery", "stop", "plan-1", "--why", "w", "--by", "mesh-delivery for jochen", + "--without", "novox/mesh-lab@b1,novox/mesh-lab@b2"} + if err != nil || !reflect.DeepEqual(got, want) { + t.Fatalf("delivery-stop with without → %v %v, wanted %v", got, err, want) + } +} + +// A walk of the catalogue's merge and another repository's is stopped without the catalogue's: it ends stopped, +// names the batch that walks the other merge again and the merge it left out, and that batch is cut into a walk +// that builds the other repository's module alone. +func TestAStopWithoutAMergeWalksTheOthersAgain(t *testing.T) { + open := windowed(t) + asksRecorded(t) + ctx := t.Context() + cat := catalogueMerge("cat1aaaaaaaa", "app", t0) + one := repoMerge("one", "one1bbbbbbbb", t0.Add(5*time.Second)) + hear(t, open, cat, t0.Add(time.Second)) + hear(t, open, one, t0.Add(6*time.Second)) + cutAt(t, open, t0.Add(2*time.Minute)) + ws, _ := walks(t, open) + if len(ws) != 1 || !ws[0].Open() { + t.Fatalf("no open walk to stop: %+v", ws) + } + stopped := ws[0] + p, said, err := stopWalkWithout(ctx, open.inventory, stopped.ID, "mesh-delivery for jochen", "its group stopped", + []string{"novox/mesh-catalog@" + cat.Commit}, t0.Add(3*time.Minute)) + if err != nil { + t.Fatal(err) + } + if p.State != inventory.PlanFailed || p.Delivery.Stopped == "" || p.Delivery.WalkedAgainBy == "" || + !reflect.DeepEqual(p.Delivery.LeftOut, []inventory.PlanLeftOut{{Repository: "novox/mesh-catalog", Commit: cat.Commit}}) { + t.Fatalf("the stopped walk reads %s %+v", p.State, p.Delivery) + } + if said.WalkedAgainBy != p.Delivery.WalkedAgainBy || !reflect.DeepEqual(said.WalkedAgain, []string{"novox/one@" + one.Commit}) { + t.Fatalf("the stop answered %+v", said) + } + again, err := open.inventory.MergesOf(ctx, p.Delivery.WalkedAgainBy) + if err != nil || len(again) != 1 || again[0].Commit != one.Commit { + t.Fatalf("the fresh batch answers %+v (%v); want the other repository's merge alone", again, err) + } + if left, _ := open.inventory.MergesOf(ctx, stopped.ID); len(left) != 1 || left[0].Commit != cat.Commit { + t.Fatalf("the stopped walk still answers %+v; want the left-out merge alone", left) + } + cutAt(t, open, t0.Add(4*time.Minute)) + fresh, err := open.inventory.PlanByID(ctx, p.Delivery.WalkedAgainBy) + if err != nil || fresh.Batch() || !fresh.Open() { + t.Fatalf("the fresh batch was not cut into a walk: %s %v", fresh.State, err) + } + if _, in := fresh.Modules["one"]; !in { + t.Fatalf("the fresh walk does not build one: %v", fresh.Modules) + } + if _, in := fresh.Modules["app"]; in { + t.Fatalf("the fresh walk builds app, whose merge was left out: %v", fresh.Modules) + } + if len(fresh.Delivery.Merges) != 1 || fresh.Delivery.Merges[0].Commit != one.Commit { + t.Fatalf("the fresh walk answers %+v", fresh.Delivery.Merges) + } + // A stopped walk starts no search of its own (ADR 0276 decision 3 is for a failed one). + cutAt(t, open, t0.Add(5*time.Minute)) + if alone, _ := open.inventory.AloneMerges(ctx); len(alone) != 0 { + t.Fatalf("a merge of the stopped walk waits to be walked alone: %+v", alone) + } +} + +// A later merge of the stopped merge's repository contains it: it is left out too, named with the merge it +// contains, and only the other repository's merge is walked again. With nothing left to walk, the walk is only +// ended. A merge the walk does not answer is refused, and nothing ends. +func TestAStopWithoutAMergeEndsTheMergesThatContainIt(t *testing.T) { + open := windowed(t) + asksRecorded(t) + ctx := t.Context() + first := catalogueMerge("cat1aaaaaaaa", "app", t0) + later := catalogueMerge("cat2cccccccc", "notes", t0.Add(10*time.Second)) + one := repoMerge("one", "one1bbbbbbbb", t0.Add(5*time.Second)) + hear(t, open, first, t0.Add(time.Second)) + hear(t, open, one, t0.Add(6*time.Second)) + hear(t, open, later, t0.Add(11*time.Second)) + cutAt(t, open, t0.Add(2*time.Minute)) + ws, _ := walks(t, open) + walk := ws[0] + + if _, _, err := stopWalkWithout(ctx, open.inventory, walk.ID, "mesh-delivery for jochen", "x", + []string{"novox/mesh-catalog@ffffffffffff"}, t0.Add(3*time.Minute)); err == nil { + t.Fatal("a stop leaving out a merge the walk does not answer was taken") + } + if p, _ := open.inventory.PlanByID(ctx, walk.ID); !p.Open() { + t.Fatalf("a refused stop ended the walk: %s", p.State) + } + + p, said, err := stopWalkWithout(ctx, open.inventory, walk.ID, "mesh-delivery for jochen", "its group stopped", + []string{"novox/mesh-catalog@" + first.Commit}, t0.Add(3*time.Minute)) + if err != nil { + t.Fatal(err) + } + want := []inventory.PlanLeftOut{{Repository: "novox/mesh-catalog", Commit: first.Commit}, + {Repository: "novox/mesh-catalog", Commit: later.Commit, Contains: first.Commit}} + if !reflect.DeepEqual(p.Delivery.LeftOut, want) || !reflect.DeepEqual(said.LeftOut, want) { + t.Fatalf("left out %+v, said %+v; want %+v", p.Delivery.LeftOut, said.LeftOut, want) + } + if !strings.Contains(p.Note, later.Commit[:8]) || !strings.Contains(p.Note, p.Delivery.WalkedAgainBy) { + t.Fatalf("the stopped walk's note does not name the merge ended with it and the fresh batch: %q", p.Note) + } + again, _ := open.inventory.MergesOf(ctx, p.Delivery.WalkedAgainBy) + if len(again) != 1 || again[0].Commit != one.Commit { + t.Fatalf("walked again: %+v; want the other repository's merge alone", again) + } + + // Nothing left once the other repository's merge is left out too: the fresh walk is stopped, and nothing more. + cutAt(t, open, t0.Add(4*time.Minute)) + fresh, _ := open.inventory.PlanByID(ctx, p.Delivery.WalkedAgainBy) + q, said, err := stopWalkWithout(ctx, open.inventory, fresh.ID, "mesh-delivery for jochen", "that one too", + []string{"novox/one@" + one.Commit}, t0.Add(5*time.Minute)) + if err != nil || q.State != inventory.PlanFailed || q.Delivery.WalkedAgainBy != "" || said.WalkedAgainBy != "" { + t.Fatalf("a stop leaving nothing to walk: %s %+v %+v %v", q.State, q.Delivery, said, err) + } + if _, bs := walks(t, open); len(bs) != 0 { + t.Fatalf("a batch of nothing was made: %+v", bs) + } +} diff --git a/cmd/mesh-controller/seatverbs.go b/cmd/mesh-controller/seatverbs.go index 81838973..ac0030d0 100644 --- a/cmd/mesh-controller/seatverbs.go +++ b/cmd/mesh-controller/seatverbs.go @@ -410,6 +410,10 @@ func (a *verbArguments) commandLine() ([]string, error) { if b := str("by"); b != "" { argv = append(argv, "--by", catalogue.DeliverySeat+" for "+b) } + // The merges to leave out: the walk's other merges are walked again without them (novox/hq ADR 0299). + if w := str("without"); w != "" { + argv = append(argv, "--without", w) + } return argv, nil case "delivery-walks": argv := []string{"delivery", "walks"} diff --git a/internal/catalogue/verbs.go b/internal/catalogue/verbs.go index f3aaedaf..965f06bf 100644 --- a/internal/catalogue/verbs.go +++ b/internal/catalogue/verbs.go @@ -160,8 +160,12 @@ var ControllerVerbs = []Verb{ Input: schema(map[string]string{"plan": "the walk's id", "why": "what lets it go: its turn, a person's release"}, []string{"plan"})}, {Name: "delivery-stop", Description: "End a walk on its delivery's word (novox/hq ADR 0239): failed, said as stopped " + - "by whom and why; what it asked still builds and registers, nothing further is asked or sent.", - Input: schema(map[string]string{"plan": "the walk's id", "why": "why", "by": "who stopped the delivery"}, + "by whom and why; what it asked still builds and registers, nothing further is asked or sent. Given merges to " + + "leave out, every other merge the walk answered is walked again in a fresh batch without them and without each " + + "later merge of their repository, which contains one and is left out too, named (novox/hq ADR 0299); the " + + "answer names the batch, the merges walked again and the merges left out.", + Input: schema(map[string]string{"plan": "the walk's id", "why": "why", "by": "who stopped the delivery", + "without": "merges to leave out, @, comma-separated"}, []string{"plan", "why"})}, {Name: "delivery-walks", Description: "The walks the controller keeps (novox/hq ADR 0239): every open one and the " + "last ended ones, each whole — its tiers, each module's state, first machines and first-node gate with its readings, and its " + diff --git a/internal/inventory/plans.go b/internal/inventory/plans.go index bd03edc5..cbc20f3f 100644 --- a/internal/inventory/plans.go +++ b/internal/inventory/plans.go @@ -187,6 +187,11 @@ type PlanDelivery struct { // TakenOverBy names the walk a walk folded before it started was taken over by: its merges are that // walk's now. TakenOverBy string `json:"taken_over_by,omitempty"` + // WalkedAgainBy names, on a walk stopped without some of its merges, the batch that walks its other merges + // again, and LeftOut the merges it left out (novox/hq issue 459, ADR 0299): a delivery of a merge left out + // ends stopped with the walk, one of a merge walked again follows that batch's walk. + WalkedAgainBy string `json:"walked_again_by,omitempty"` + LeftOut []PlanLeftOut `json:"left_out,omitempty"` // Batch is the window of a batch; nil once it is cut into a walk. Batch *PlanBatch `json:"batch,omitempty"` // Alone says the walk walks one merge on its own commit, after a failed walk carried it in a later one: @@ -194,6 +199,16 @@ type PlanDelivery struct { Alone bool `json:"alone,omitempty"` } +// PlanLeftOut is a merge a stop left out of the batch that walks a stopped walk's merges again (novox/hq ADR +// 0299): a merge stopped, or a later merge of its repository's branch, which contains the stopped one and so +// cannot be walked without delivering it. +type PlanLeftOut struct { + Repository string `json:"repository"` + Commit string `json:"commit"` + // Contains is the stopped merge a later merge contains; empty for a merge stopped itself. + Contains string `json:"contains,omitempty"` +} + // Waiting is whether the walk waits for its delivery's word. func (p Plan) Waiting() bool { return p.Delivery != nil && p.Delivery.Awaits != "" && p.Delivery.Go == nil From 309475bf6faba659bd4e1a1d9bcb109d4c4ebc3a Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 11 Oct 2026 20:12:27 +0200 Subject: [PATCH 2/5] Walk a stopped walk's merges again in the open batch, in one act, and say what stays on the machines (review of #232, hq issue 459) A new batch beside the open one walked an older commit after a newer one, and a stop that failed partway could leave a merge on two plans. A module shared with the stopped merge, built at its branch, would deliver it; it is left out or withheld. A batch not yet cut can drop a merge, so mesh-delivery can stop a merge walked again. The stopped walk names what it left on which machines. --- cmd/mesh-controller/batches.go | 52 +++- cmd/mesh-controller/delivery.go | 408 ++++++++++++++++++++------- cmd/mesh-controller/issue459_test.go | 216 ++++++++++++++ cmd/mesh-controller/plan_retry.go | 7 + internal/catalogue/verbs.go | 8 +- internal/inventory/plans.go | 157 +++++++---- merge-check.sh | 15 + 7 files changed, 715 insertions(+), 148 deletions(-) diff --git a/cmd/mesh-controller/batches.go b/cmd/mesh-controller/batches.go index 291a6718..a0db5821 100644 --- a/cmd/mesh-controller/batches.go +++ b/cmd/mesh-controller/batches.go @@ -367,7 +367,13 @@ func keepBatch(ctx context.Context, inv *inventory.Inventory, b *inventory.Plan, } closes, latest := windowOf(merges, b.Created, window, atMost) b.Delivery.Merges = named - b.Delivery.Batch = &inventory.PlanBatch{ClosesAt: closes, AtMost: latest, Behind: behind, Own: b.OwnPath()} + var closed bool + var withheld []string + if was := b.Delivery.Batch; was != nil { + closed, withheld = was.Closed, was.Withheld // a stop's, which no look takes back (novox/hq ADR 0299) + } + b.Delivery.Batch = &inventory.PlanBatch{ClosesAt: closes, AtMost: latest, Behind: behind, Own: b.OwnPath(), + Closed: closed, Withheld: withheld} b.State = inventory.PlanAssembling if behind != "" && windowClosed(*b, now) { b.State = inventory.PlanQueued @@ -402,7 +408,7 @@ func windowClosed(b inventory.Plan, now time.Time) bool { return true } w := b.Delivery.Batch - return !now.Before(w.ClosesAt) || !now.Before(w.AtMost) + return w.Closed || !now.Before(w.ClosesAt) || !now.Before(w.AtMost) } // carriedOf is a set of merges as a walk carries them: the latest merge of each repository's branch, and @@ -763,6 +769,15 @@ func planBatch(ctx context.Context, open *stores, batch *inventory.Plan, carry [ sort.Strings(also) ordered := groupOrderEdges(members, entries, read, edges) plan := planOfMoves(moved, append(append([]inventory.Edge{}, edges...), ordered...)) + // **Withheld, a stopped merge's modules** (novox/hq ADR 0299): a module is built at its branch, which holds the + // merge a stop left out of the walk these merges were walked again from — built here only as a dependent, it + // would deliver that merge. One a merge of this batch moves is built: that merge carries the stopped change, + // and the stop named it. + if batch.Delivery != nil && batch.Delivery.Batch != nil { + if out := withhold(&plan, batch.Delivery.Batch.Withheld, moved); len(out) > 0 { + fmt.Printf(" withheld, a stopped merge's modules built only as dependents: %s\n", strings.Join(out, ", ")) + } + } plan.ID, plan.Revision, plan.Created, plan.Epoch = batch.ID, batch.Revision, now, batch.Epoch plan.Commits = commits newest := newestCommit(commits) @@ -823,6 +838,39 @@ func planBatch(ctx context.Context, open *stores, batch *inventory.Plan, carry [ return nil } +// withhold takes from a plan every module named that its merges do not move, and the tiers left empty: the +// modules taken. +func withhold(plan *inventory.Plan, names, moved []string) []string { + var out []string + for _, n := range names { + if _, in := plan.Modules[n]; in && !slices.Contains(moved, n) { + delete(plan.Modules, n) + out = append(out, n) + } + } + if len(out) == 0 { + return nil + } + var tiers [][]string + for _, t := range plan.Tiers { + var kept []string + for _, n := range t { + if !slices.Contains(out, n) { + kept = append(kept, n) + } + } + if len(kept) > 0 { + tiers = append(tiers, kept) + } + } + if tiers == nil { + tiers = [][]string{} + } + plan.Tiers = tiers + sort.Strings(out) + return out +} + // walkTimesAtCut is a walk's own moments as it is cut (novox/hq ADR 0282 decision 6): when its batch's window // closed — no merge for the window's length, or its maximum, whichever came first — when it was cut, and its // class. A merge walked alone had no window. diff --git a/cmd/mesh-controller/delivery.go b/cmd/mesh-controller/delivery.go index 1a4d7400..f19ee10b 100644 --- a/cmd/mesh-controller/delivery.go +++ b/cmd/mesh-controller/delivery.go @@ -121,22 +121,35 @@ func stopWalk(ctx context.Context, inv *inventory.Inventory, id, by, why string) return p, err } -// stopAnswer is what a stop that leaves merges out says, as data (novox/hq ADR 0299): the walk stopped, the -// batch that walks its other merges again, those merges as `@`, and every merge left out. +// stopAnswer is what a stop that leaves merges out says, as data (novox/hq ADR 0299): the record stopped, the +// batch that walks its other merges again with those merges, every merge left out, the modules the stopped walk +// already sent that stay on the machines named, the modules the batch's walk withholds, and every merge elsewhere +// that will deliver a stopped change when it is walked — a later merge of its repository, or one moving its modules. type stopAnswer struct { - Stopped string `json:"stopped"` - WalkedAgainBy string `json:"walked_again_by,omitempty"` - WalkedAgain []string `json:"walked_again,omitempty"` - LeftOut []inventory.PlanLeftOut `json:"left_out,omitempty"` + Stopped string `json:"stopped"` + WalkedAgainBy string `json:"walked_again_by,omitempty"` + WalkedAgain []string `json:"walked_again,omitempty"` + LeftOut []inventory.PlanLeftOut `json:"left_out,omitempty"` + Kept []inventory.PlanKept `json:"kept,omitempty"` + Withheld []string `json:"withheld,omitempty"` + StillCarriedBy []string `json:"still_carried_by,omitempty"` } // stopWalkWithout ends a walk on its delivery's word and, given merges to leave out, walks every other merge it -// answered again in a fresh batch without them (novox/hq issue 459, ADR 0299): one walk carries every merge of its -// batch, so ending it for one group member ended every other delivery on it. Left out are the merges named and -// each later merge of their repository's branch, which contains one of them and cannot be walked without -// delivering it. The fresh batch's window is closed — its merges were heard before — and it is cut by the usual -// rules, as a failed walk's merges are walked again (ADR 0276 decision 3). With nothing left to walk, or no merge -// named, the walk is only ended. A merge named that the walk does not answer refuses the stop whole. +// answered again without them (novox/hq issue 459, ADR 0299): one walk carries every merge of its batch, so ending +// it for one group member ended every other delivery on it. +// +// - Left out are the merges named, each later merge of their repository's branch, which contains one, and each +// merge moving a module a left-out merge moves: a module is built at its branch, which holds the stopped merge. +// - The others join the open batch of their kind — merges heard while the walk ran wait there, and an older +// commit never walks after a newer one — or a new batch when none is open; its window is closed, and its walk +// withholds the left-out merges' modules where it would build them only as dependents. +// - Given a batch not yet cut, the merges left out leave it for a stopped record of their own, and the batch +// goes on without them; with nothing left in it, the batch itself is stopped. +// - The stopped record, the batch and every merge moved are written in one transaction (decision 7). +// +// With nothing left to walk, or no merge named, the walk is only ended. A merge named that the record does not +// answer refuses the stop whole. func stopWalkWithout(ctx context.Context, inv *inventory.Inventory, id, by, why string, without []string, now time.Time) (inventory.Plan, stopAnswer, error) { said := stopAnswer{Stopped: id} @@ -149,94 +162,158 @@ func stopWalkWithout(ctx context.Context, inv *inventory.Inventory, id, by, why if err != nil { return inventory.Plan{}, said, err } - if !p.Open() { + batch := p.Batch() + if !p.Open() && !(batch && len(without) > 0) { return p, said, fmt.Errorf("%s is already %s", p.ID, p.State) } - var again []inventory.BatchedMerge + var merges, again []inventory.BatchedMerge if len(without) > 0 { - merges, err := inv.MergesOf(ctx, p.ID) - if err != nil { + if merges, err = inv.MergesOf(ctx, p.ID); err != nil { return p, said, err } if again, said.LeftOut, err = leaveOut(merges, without); err != nil { return p, said, fmt.Errorf("%s is not stopped: %w", p.ID, err) } } - if p.Delivery == nil { - p.Delivery = &inventory.PlanDelivery{} - } - p.Delivery.Stopped, p.Delivery.StoppedWhy = by, why - p.Delivery.LeftOut = said.LeftOut - p.State = inventory.PlanFailed - p.Note = fmt.Sprintf("stopped by %s at tier %d: %s", by, p.Tier, why) - if len(again) > 0 { - batch, err := walkAgain(ctx, inv, p, again, now) - if err != nil { - return p, said, fmt.Errorf("%s is not stopped: its other merges could not be batched again: %w", p.ID, err) + stopped := func(r *inventory.Plan) { + if r.Delivery == nil { + r.Delivery = &inventory.PlanDelivery{} } - said.WalkedAgainBy = batch.ID - for _, m := range again { - said.WalkedAgain = append(said.WalkedAgain, m.Repository+"@"+m.Commit) - } - p.Delivery.WalkedAgainBy = batch.ID - p.Note += fmt.Sprintf("; its other merges are walked again without %s, by %s: %s", mergesLeftOutWords(said.LeftOut), - batch.ID, strings.Join(said.WalkedAgain, ", ")) - if err := inv.SavePlan(ctx, &p); err != nil { - // The merges go back to the walk that still answers them, and the batch holds none: it is never walked. - for _, m := range again { - _ = inv.AnswerMerge(ctx, m.Repository, m.Commit, p.ID, m.Alone) - } - return p, said, err - } - fmt.Printf("%s stopped by %s; %s walks its other merges again: %s\n", p.ID, by, batch.ID, - strings.Join(said.WalkedAgain, ", ")) - return p, said, nil + r.Delivery.Stopped, r.Delivery.StoppedWhy, r.Delivery.LeftOut = by, why, said.LeftOut + r.State = inventory.PlanFailed + r.Note = fmt.Sprintf("stopped by %s at tier %d: %s", by, r.Tier, why) } - if len(said.LeftOut) > 0 { - p.Note += fmt.Sprintf("; left out %s, nothing else to walk", mergesLeftOutWords(said.LeftOut)) - } - if err := inv.SavePlan(ctx, &p); err != nil { + withheld := movesOf(leftOutMerges(merges, said.LeftOut)) + said.Withheld = withheld + if said.StillCarriedBy, err = stillCarried(ctx, inv, p.ID, merges, said.LeftOut, withheld); err != nil { return p, said, err } + if batch { + return p, said, stopInBatch(ctx, inv, &p, merges, again, &said, stopped, now) + } + said.Kept = keptOf(p, movesOf(again)) + stopped(&p) + p.Delivery.Kept = said.Kept + if len(again) == 0 { + if len(said.LeftOut) > 0 { + p.Note += fmt.Sprintf("; left out %s, nothing else to walk", mergesLeftOutWords(said.LeftOut)) + } + return p, said, inv.SavePlan(ctx, &p) + } + target, err := batchToWalkAgain(ctx, inv, again, now) + if err != nil { + return p, said, fmt.Errorf("%s is not stopped: %w", p.ID, err) + } + target.Delivery.Batch.Closed = true + target.Delivery.Batch.Withheld = union(target.Delivery.Batch.Withheld, withheld) + sort.Strings(target.Delivery.Batch.Withheld) + said.WalkedAgainBy = target.ID + var moves []inventory.MergeMove + for _, m := range again { + said.WalkedAgain = append(said.WalkedAgain, m.Repository+"@"+m.Commit) + moves = append(moves, inventory.MergeMove{Repository: m.Repository, Commit: m.Commit, Plan: target.ID, Alone: m.Alone}) + } + p.Delivery.WalkedAgainBy = target.ID + p.Note += fmt.Sprintf("; its other merges are walked again by %s: %s", target.ID, strings.Join(said.WalkedAgain, ", ")) + if len(said.LeftOut) > 0 { + p.Note += "; left out " + mergesLeftOutWords(said.LeftOut) + } + if len(said.Kept) > 0 { + p.Note += "; " + keptWords(said.Kept) + } + // The batch first, so no merge names a plan the store does not hold; all of it, or nothing (decision 7). + if err := inv.SavePlansMoving(ctx, []*inventory.Plan{&target, &p}, moves); err != nil { + return p, said, fmt.Errorf("%s is not stopped, and none of its merges moved: %w", p.ID, err) + } + // What the batch holds and names now: written again by the cutter's next look should this fail. + if err := keepBatch(ctx, inv, &target, now, ""); err != nil { + fmt.Printf(" %s holds its merges; its record is written again at the next look: %v\n", target.ID, err) + } + fmt.Printf("%s stopped by %s; %s walks its other merges again: %s\n", p.ID, by, target.ID, + strings.Join(said.WalkedAgain, ", ")) return p, said, nil } -// walkAgain puts merges of a stopped walk into a fresh batch of their kind, its window closed: kept before its -// first merge names it, then each merge moved to it. -func walkAgain(ctx context.Context, inv *inventory.Inventory, stopped inventory.Plan, again []inventory.BatchedMerge, +// stopInBatch leaves merges out of a batch not yet cut (ADR 0299): they go to a stopped record of their own, which +// names the batch as walking the others, and the batch withholds their modules; a batch left with none is stopped +// itself. +func stopInBatch(ctx context.Context, inv *inventory.Inventory, b *inventory.Plan, merges, again []inventory.BatchedMerge, + said *stopAnswer, stopped func(*inventory.Plan), now time.Time) error { + if len(again) == 0 { + stopped(b) + b.Delivery.Batch = nil + b.Note = "a batch whose every merge was left out by a stop: " + b.Note + return inv.SavePlan(ctx, b) + } + left := leftOutMerges(merges, said.LeftOut) + first := left[0] + r := inventory.Plan{ID: fmt.Sprintf("plan-%d", now.UnixNano()), Repository: first.Repository, Branch: first.Branch, + Commit: first.Commit, Merged: first.Merged, Created: now, Tiers: [][]string{}, + Modules: map[string]*inventory.PlanModule{}} + if r.ID == b.ID { + r.ID += "-left" + } + stopped(&r) + r.Delivery.WalkedAgainBy = b.ID + r.Note += fmt.Sprintf("; left out of %s before it was walked: %s; %s walks the others", b.ID, + mergesLeftOutWords(said.LeftOut), b.ID) + b.Delivery.Batch.Withheld = union(b.Delivery.Batch.Withheld, said.Withheld) + sort.Strings(b.Delivery.Batch.Withheld) + var moves []inventory.MergeMove + for _, m := range left { + moves = append(moves, inventory.MergeMove{Repository: m.Repository, Commit: m.Commit, Plan: r.ID, Alone: m.Alone}) + } + for _, m := range again { + said.WalkedAgain = append(said.WalkedAgain, m.Repository+"@"+m.Commit) + } + said.Stopped, said.WalkedAgainBy = r.ID, b.ID + if err := inv.SavePlansMoving(ctx, []*inventory.Plan{&r, b}, moves); err != nil { + return fmt.Errorf("%s is not changed, and none of its merges moved: %w", b.ID, err) + } + if err := keepBatch(ctx, inv, b, now, ""); err != nil { + fmt.Printf(" %s holds its merges; its record is written again at the next look: %v\n", b.ID, err) + } + fmt.Printf("%s: %s left out by a stop, kept by %s\n", b.ID, mergesLeftOutWords(said.LeftOut), r.ID) + return nil +} + +// batchToWalkAgain is the batch a stopped walk's merges join: the open batch of their kind, or a new one, not yet +// written. The kind is read from what the merges moved. +func batchToWalkAgain(ctx context.Context, inv *inventory.Inventory, again []inventory.BatchedMerge, now time.Time) (inventory.Plan, error) { own := false - for _, m := range again { - for _, n := range announcedOf(m).Moves { - if _, on := onTheControllersPath[n]; on { - own = true + for _, n := range movesOf(again) { + if _, on := onTheControllersPath[n]; on { + own = true + } + } + batches, err := inv.Batches(ctx) + if err != nil { + return inventory.Plan{}, err + } + for _, b := range batches { + if b.OwnPath() == own { + if b.Delivery == nil { + b.Delivery = &inventory.PlanDelivery{} } + if b.Delivery.Batch == nil { + b.Delivery.Batch = &inventory.PlanBatch{Own: own} + } + return b, nil } } first := again[0] - b := inventory.Plan{ID: fmt.Sprintf("plan-%d", now.UnixNano()), Repository: first.Repository, Branch: first.Branch, + return inventory.Plan{ID: fmt.Sprintf("plan-%d", now.UnixNano()), Repository: first.Repository, Branch: first.Branch, Commit: first.Commit, Merged: first.Merged, Created: now, State: inventory.PlanAssembling, Tiers: [][]string{}, Modules: map[string]*inventory.PlanModule{}, - Delivery: &inventory.PlanDelivery{Batch: &inventory.PlanBatch{Own: own}}} - if b.ID == stopped.ID { - b.ID += "-again" - } - if err := inv.SavePlan(ctx, &b); err != nil { - return b, err - } - for _, m := range again { - if err := inv.AnswerMerge(ctx, m.Repository, m.Commit, b.ID, m.Alone); err != nil { - return b, err - } - } - // Its merges were heard before: the window, measured from the last of them, is closed, and the cutter cuts it - // once no walk of its kind is open. - return b, keepBatch(ctx, inv, &b, now, "") + Delivery: &inventory.PlanDelivery{Batch: &inventory.PlanBatch{Own: own}}}, nil } // leaveOut splits a walk's merges by the merges named to leave out, each `@` (a commit's -// first characters will do): the merges walked again, and those left out — each named one, and each later merge -// of its repository's branch, which contains it. A name the walk does not answer is refused. +// first characters will do): the merges walked again, and those left out — each named one; each later merge of +// its repository's branch, which contains it; and each merge moving a module a left-out merge moves, which is +// built at its branch and so holds the stopped merge — until none more is. A name the walk does not answer is +// refused. func leaveOut(merges []inventory.BatchedMerge, without []string) ([]inventory.BatchedMerge, []inventory.PlanLeftOut, error) { var stopped []inventory.BatchedMerge for _, w := range without { @@ -252,40 +329,181 @@ func leaveOut(merges []inventory.BatchedMerge, without []string) ([]inventory.Ba } stopped = append(stopped, merges[i]) } - // The earliest stopped merge of each repository's branch: every merge of that branch after it contains it. - earliest := map[string]inventory.BatchedMerge{} - for _, s := range stopped { - k := strings.ToLower(s.Repository) + "\x00" + s.Branch - if e, ok := earliest[k]; !ok || laterMerge(e, s) { - earliest[k] = s + same := func(a, b inventory.BatchedMerge) bool { + return strings.EqualFold(a.Repository, b.Repository) && a.Commit == b.Commit + } + out := map[int]inventory.PlanLeftOut{} + for i, m := range merges { + if slices.ContainsFunc(stopped, func(s inventory.BatchedMerge) bool { return same(s, m) }) { + out[i] = inventory.PlanLeftOut{Repository: m.Repository, Commit: m.Commit} + } + } + for grew := true; grew; { + grew = false + for i, m := range merges { + if _, left := out[i]; left { + continue + } + for j, o := range out { + l := merges[j] + contains := o.Contains + if contains == "" { + contains = l.Commit + } + if strings.EqualFold(l.Repository, m.Repository) && l.Branch == m.Branch && laterMerge(m, l) { + out[i] = inventory.PlanLeftOut{Repository: m.Repository, Commit: m.Commit, Contains: contains} + } else if shared := sharedMoves(m, l); len(shared) > 0 { + out[i] = inventory.PlanLeftOut{Repository: m.Repository, Commit: m.Commit, Contains: contains, + Shares: shared} + } else { + continue + } + grew = true + break + } } } var again []inventory.BatchedMerge - var out []inventory.PlanLeftOut - for _, m := range merges { - named := slices.ContainsFunc(stopped, func(s inventory.BatchedMerge) bool { - return strings.EqualFold(s.Repository, m.Repository) && s.Commit == m.Commit - }) - e, has := earliest[strings.ToLower(m.Repository)+"\x00"+m.Branch] - switch { - case named: - out = append(out, inventory.PlanLeftOut{Repository: m.Repository, Commit: m.Commit}) - case has && laterMerge(m, e): - out = append(out, inventory.PlanLeftOut{Repository: m.Repository, Commit: m.Commit, Contains: e.Commit}) - default: + var left []inventory.PlanLeftOut + for i, m := range merges { + if o, is := out[i]; is { + left = append(left, o) + } else { again = append(again, m) } } - sort.SliceStable(out, func(i, j int) bool { return out[i].Contains == "" && out[j].Contains != "" }) - return again, out, nil + sort.SliceStable(left, func(i, j int) bool { return left[i].Contains == "" && left[j].Contains != "" }) + return again, left, nil } -// mergesLeftOutWords is the merges a stop left out, as a person reads them: each that contains a stopped one says so. +// sharedMoves are the modules two kept merges both moved when they were heard. +func sharedMoves(a, b inventory.BatchedMerge) []string { + var out []string + theirs := announcedOf(b).Moves + for _, n := range announcedOf(a).Moves { + if slices.Contains(theirs, n) && !slices.Contains(out, n) { + out = append(out, n) + } + } + sort.Strings(out) + return out +} + +// leftOutMerges are the kept merges a stop left out. +func leftOutMerges(merges []inventory.BatchedMerge, out []inventory.PlanLeftOut) []inventory.BatchedMerge { + var left []inventory.BatchedMerge + for _, m := range merges { + if slices.ContainsFunc(out, func(o inventory.PlanLeftOut) bool { + return strings.EqualFold(o.Repository, m.Repository) && o.Commit == m.Commit + }) { + left = append(left, m) + } + } + return left +} + +// movesOf is every module kept merges moved when they were heard, sorted. +func movesOf(merges []inventory.BatchedMerge) []string { + var out []string + for _, m := range merges { + out = union(out, announcedOf(m).Moves) + } + sort.Strings(out) + return out +} + +// keptOf are the modules a stopped walk sent that no merge walked again moves, and the machines each reached: +// those keep running the stopped walk's build. +func keptOf(p inventory.Plan, walkedAgain []string) []inventory.PlanKept { + var out []inventory.PlanKept + for name, m := range p.Modules { + if m == nil || slices.Contains(walkedAgain, name) { + continue + } + var machines []string + if m.FirstAt != nil || m.SentAt != nil { + machines = union(machines, m.First) + } + if m.Gate != nil && m.Gate.Since != nil { + machines = union(machines, m.Gate.Machines) + } + for machine := range m.Rest { + machines = union(machines, []string{machine}) + } + if len(machines) == 0 { + continue + } + sort.Strings(machines) + out = append(out, inventory.PlanKept{Module: name, Machines: machines}) + } + sort.Slice(out, func(i, j int) bool { return out[i].Module < out[j].Module }) + return out +} + +// keptWords is what a stopped walk leaves on the machines, as a person reads it. +func keptWords(kept []inventory.PlanKept) string { + var words []string + for _, k := range kept { + words = append(words, k.Module+" on "+strings.Join(k.Machines, ", ")) + } + return "left running the stopped walk's build, which nothing walks again: " + strings.Join(words, "; ") +} + +// stillCarried are the merges outside the stopped record, in a batch or an open walk, that will deliver a stopped +// change when they are walked: a later merge of its repository's branch, which contains it, or one that moves a +// module it moves. Holding a merged change back for good is a revert's (ADR 0299). +func stillCarried(ctx context.Context, inv *inventory.Inventory, id string, merges []inventory.BatchedMerge, + out []inventory.PlanLeftOut, withheld []string) ([]string, error) { + var named []inventory.BatchedMerge + for _, m := range leftOutMerges(merges, out) { + if slices.ContainsFunc(out, func(o inventory.PlanLeftOut) bool { return o.Commit == m.Commit && o.Contains == "" }) { + named = append(named, m) + } + } + if len(named) == 0 { + return nil, nil + } + open, err := inv.OpenPlans(ctx) + if err != nil { + return nil, err + } + batches, err := inv.Batches(ctx) + if err != nil { + return nil, err + } + var carried []string + for _, p := range append(open, batches...) { + if p.ID == id { + continue + } + ms, err := inv.MergesOf(ctx, p.ID) + if err != nil { + return nil, err + } + for _, m := range ms { + later := slices.ContainsFunc(named, func(s inventory.BatchedMerge) bool { + return strings.EqualFold(s.Repository, m.Repository) && s.Branch == m.Branch && laterMerge(m, s) + }) + shares := slices.ContainsFunc(announcedOf(m).Moves, func(n string) bool { return slices.Contains(withheld, n) }) + if later || shares { + carried = append(carried, fmt.Sprintf("%s@%s (in %s)", m.Repository, m.Commit, p.ID)) + } + } + } + sort.Strings(carried) + return carried, nil +} + +// mergesLeftOutWords is the merges a stop left out, as a person reads them: each that contains a stopped one, or +// shares its modules, says so. func mergesLeftOutWords(out []inventory.PlanLeftOut) string { var words []string for _, o := range out { s := o.Repository + "@" + short(o.Commit) - if o.Contains != "" { + switch { + case len(o.Shares) > 0: + s += " (which moves " + strings.Join(o.Shares, ", ") + " as " + short(o.Contains) + " does)" + case o.Contains != "": s += " (which contains " + short(o.Contains) + ")" } words = append(words, s) diff --git a/cmd/mesh-controller/issue459_test.go b/cmd/mesh-controller/issue459_test.go index c5591b3d..b57accc3 100644 --- a/cmd/mesh-controller/issue459_test.go +++ b/cmd/mesh-controller/issue459_test.go @@ -1,7 +1,10 @@ package main import ( + "encoding/json" + "errors" "reflect" + "slices" "strings" "testing" "time" @@ -52,6 +55,49 @@ func TestLeftOutAreTheStoppedMergeAndTheMergesThatContainIt(t *testing.T) { } } +// moving is a kept merge that moved modules when it was heard. +func moving(m inventory.BatchedMerge, modules ...string) inventory.BatchedMerge { + m.Event, _ = json.Marshal(keptMerge{Moves: modules}) + return m +} + +// A merge of another repository that moves a module the stopped merge moves is left out too, naming the modules +// it shares: the module is built at its branch, which holds the stopped merge (review of #232). And so on, until +// none more is: a merge sharing a module with that one is left out as well. +func TestAMergeSharingAStoppedMergesModuleIsLeftOut(t *testing.T) { + stopped := moving(leaved("novox/mesh-catalog", "c1aaaaaaaaaa", t0), "app") + reader := moving(leaved("novox/one", "o1bbbbbbbbbb", t0.Add(time.Second)), "app", "one") + next := moving(leaved("novox/two", "t1cccccccccc", t0.Add(2*time.Second)), "one") + free := moving(leaved("novox/three", "h1dddddddddd", t0.Add(3*time.Second)), "three") + again, out, err := leaveOut([]inventory.BatchedMerge{stopped, reader, next, free}, []string{"novox/mesh-catalog@c1aaaaaaa"}) + if err != nil { + t.Fatal(err) + } + if len(again) != 1 || again[0].Commit != free.Commit { + t.Fatalf("walked again: %+v; want the merge that shares nothing alone", again) + } + want := []inventory.PlanLeftOut{{Repository: "novox/mesh-catalog", Commit: stopped.Commit}, + {Repository: "novox/one", Commit: reader.Commit, Contains: stopped.Commit, Shares: []string{"app"}}, + {Repository: "novox/two", Commit: next.Commit, Contains: stopped.Commit, Shares: []string{"one"}}} + if !reflect.DeepEqual(out, want) { + t.Fatalf("left out %+v; want %+v", out, want) + } +} + +// A walk withholds a module a stopped merge moved where it would build it only as a dependent; one a merge of its +// own batch moves is built, and empty tiers go. +func TestAWalkWithholdsAStoppedMergesDependents(t *testing.T) { + plan := inventory.Plan{Tiers: [][]string{{"one"}, {"app"}, {"notes", "app2"}}, + Modules: map[string]*inventory.PlanModule{"one": {}, "app": {}, "notes": {}, "app2": {}}} + got := withhold(&plan, []string{"app", "notes", "absent"}, []string{"notes"}) + if !reflect.DeepEqual(got, []string{"app"}) || !reflect.DeepEqual(plan.Tiers, [][]string{{"one"}, {"notes", "app2"}}) { + t.Fatalf("withheld %v, tiers %v", got, plan.Tiers) + } + if _, in := plan.Modules["app"]; in { + t.Fatal("a withheld module is still in the plan") + } +} + // The seat verb passes the merges to leave out to the command. func TestDeliveryStopTakesTheMergesToLeaveOut(t *testing.T) { got, err := argvFor("delivery-stop", map[string]any{"plan": "plan-1", "why": "w", "by": "jochen", @@ -175,3 +221,173 @@ func TestAStopWithoutAMergeEndsTheMergesThatContainIt(t *testing.T) { t.Fatalf("a batch of nothing was made: %+v", bs) } } + +// A merge heard while the walk ran waits in the open batch, at a later commit of the same repository. The merge +// walked again joins that batch, never a batch of its own that would walk the older commit after the newer one +// (review of #232): its window closes, and its walk is at the newer commit, answering both. +func TestAStopWalksTheOthersAgainInTheOpenBatch(t *testing.T) { + open := windowed(t) + asksRecorded(t) + ctx := t.Context() + cat := catalogueMerge("cat1aaaaaaaa", "app", t0) + one1 := repoMerge("one", "one1bbbbbbbb", t0.Add(5*time.Second)) + hear(t, open, cat, t0.Add(time.Second)) + hear(t, open, one1, t0.Add(6*time.Second)) + cutAt(t, open, t0.Add(2*time.Minute)) + ws, _ := walks(t, open) + walk := ws[0] + one2 := repoMerge("one", "one2cccccccc", t0.Add(3*time.Minute)) + hear(t, open, one2, t0.Add(3*time.Minute)) + _, bs := walks(t, open) + if len(bs) != 1 { + t.Fatalf("the merge heard while the walk ran is in %d batches", len(bs)) + } + waiting := bs[0] + p, said, err := stopWalkWithout(ctx, open.inventory, walk.ID, "mesh-delivery for jochen", "its group stopped", + []string{"novox/mesh-catalog@" + cat.Commit}, t0.Add(3*time.Minute+10*time.Second)) + if err != nil { + t.Fatal(err) + } + if p.Delivery.WalkedAgainBy != waiting.ID || said.WalkedAgainBy != waiting.ID { + t.Fatalf("walked again by %q (said %q); want the open batch %s", p.Delivery.WalkedAgainBy, said.WalkedAgainBy, waiting.ID) + } + if _, bs := walks(t, open); len(bs) != 1 { + t.Fatalf("%d batches; want the open one alone", len(bs)) + } + b, _ := open.inventory.PlanByID(ctx, waiting.ID) + if b.Delivery.Batch == nil || !b.Delivery.Batch.Closed || !slices.Contains(b.Delivery.Batch.Withheld, "app") { + t.Fatalf("the open batch reads %+v; want its window closed and app withheld", b.Delivery.Batch) + } + // Its window closed by the stop, not by the clock: cut at once. + cutAt(t, open, t0.Add(3*time.Minute+11*time.Second)) + fresh, _ := open.inventory.PlanByID(ctx, waiting.ID) + if fresh.Batch() || fresh.CommitOf("novox/one") != one2.Commit { + t.Fatalf("the batch is %s at %q; want it cut, walking the newer commit", fresh.State, fresh.CommitOf("novox/one")) + } + var answered []string + for _, m := range fresh.Delivery.Merges { + answered = append(answered, m.Commit+">"+m.Carried) + } + if !slices.Contains(answered, one1.Commit+">"+one2.Commit) || !slices.Contains(answered, one2.Commit+">") { + t.Fatalf("the walk answers %v; want the earlier merge carried by the later", answered) + } +} + +// A stop names what the stopped walk already sent and nothing walks again, on which machines; and a later merge of +// the stopped merge's repository that waits in a batch, which will deliver the stopped change. A stopped walk +// that walked its merges again is refused a retry, saying why. +func TestAStopNamesWhatStaysOnTheMachinesAndWhatStillCarriesIt(t *testing.T) { + open := windowed(t) + asksRecorded(t) + ctx := t.Context() + cat := catalogueMerge("cat1aaaaaaaa", "app", t0) + one1 := repoMerge("one", "one1bbbbbbbb", t0.Add(5*time.Second)) + hear(t, open, cat, t0.Add(time.Second)) + hear(t, open, one1, t0.Add(6*time.Second)) + cutAt(t, open, t0.Add(2*time.Minute)) + ws, _ := walks(t, open) + walk := ws[0] + sent := t0.Add(150 * time.Second) + for name, m := range walk.Modules { + if name == "app" || name == "one" { + m.First, m.FirstAt = []string{"anchor"}, &sent + } + } + if err := open.inventory.SavePlan(ctx, &walk); err != nil { + t.Fatal(err) + } + cat2 := catalogueMerge("cat2dddddddd", "notes", t0.Add(3*time.Minute)) + hear(t, open, cat2, t0.Add(3*time.Minute)) + _, said, err := stopWalkWithout(ctx, open.inventory, walk.ID, "mesh-delivery for jochen", "its group stopped", + []string{"novox/mesh-catalog@" + cat.Commit}, t0.Add(3*time.Minute+10*time.Second)) + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(said.Kept, []inventory.PlanKept{{Module: "app", Machines: []string{"anchor"}}}) { + t.Fatalf("kept %+v; want app on anchor alone (one is walked again)", said.Kept) + } + p, _ := open.inventory.PlanByID(ctx, walk.ID) + if !reflect.DeepEqual(p.Delivery.Kept, said.Kept) || !strings.Contains(p.Note, "app on anchor") { + t.Fatalf("the stopped walk's record does not name what stays: %+v %q", p.Delivery.Kept, p.Note) + } + if len(said.StillCarriedBy) != 1 || !strings.Contains(said.StillCarriedBy[0], cat2.Commit) { + t.Fatalf("still carried by %v; want the later catalogue merge waiting in the batch", said.StillCarriedBy) + } + if _, err := retryPlan(ctx, open, walk.ID); err == nil || !strings.Contains(err.Error(), "was stopped without") { + t.Fatalf("a walk stopped without some merges was retried, or refused for another reason: %v", err) + } +} + +// A batch not yet cut is stopped without a merge: the merge goes to a stopped record of its own, the batch goes on +// with the others and withholds the stopped merge's modules; with every merge left out, the batch is stopped. +func TestAStopWithoutAMergeLeavesItOutOfABatch(t *testing.T) { + open := windowed(t) + asksRecorded(t) + ctx := t.Context() + cat := catalogueMerge("cat1aaaaaaaa", "app", t0) + one := repoMerge("one", "one1bbbbbbbb", t0.Add(5*time.Second)) + hear(t, open, cat, t0.Add(time.Second)) + hear(t, open, one, t0.Add(6*time.Second)) + _, bs := walks(t, open) + b := bs[0] + r, said, err := stopWalkWithout(ctx, open.inventory, b.ID, "mesh-delivery for jochen", "its group stopped", + []string{"novox/mesh-catalog@" + cat.Commit}, t0.Add(10*time.Second)) + if err != nil { + t.Fatal(err) + } + if said.WalkedAgainBy != b.ID || said.Stopped == b.ID { + t.Fatalf("said %+v; want a stopped record of its own, and the batch walking the others", said) + } + if left, _ := open.inventory.MergesOf(ctx, said.Stopped); len(left) != 1 || left[0].Commit != cat.Commit { + t.Fatalf("the stopped record holds %+v", left) + } + if stopped, _ := open.inventory.PlanByID(ctx, said.Stopped); stopped.State != inventory.PlanFailed || + stopped.Delivery.Stopped == "" || stopped.Delivery.WalkedAgainBy != b.ID { + t.Fatalf("the stopped record reads %s %+v", stopped.State, stopped.Delivery) + } + _ = r + cutAt(t, open, t0.Add(3*time.Minute)) + w, _ := open.inventory.PlanByID(ctx, b.ID) + if _, in := w.Modules["app"]; in || w.Batch() { + t.Fatalf("the batch's walk is %s and builds %v; want it cut without app", w.State, w.Modules) + } + // A batch whose every merge is left out is stopped itself. + hear(t, open, catalogueMerge("cat3eeeeeeee", "notes", t0.Add(4*time.Minute)), t0.Add(4*time.Minute)) + _, bs = walks(t, open) + last := bs[0] + if _, said, err := stopWalkWithout(ctx, open.inventory, last.ID, "mesh-delivery for jochen", "x", + []string{"novox/mesh-catalog@cat3eeeeeeee"}, t0.Add(4*time.Minute+time.Second)); err != nil || said.Stopped != last.ID { + t.Fatalf("a batch left with nothing: %+v %v", said, err) + } + if p, _ := open.inventory.PlanByID(ctx, last.ID); p.State != inventory.PlanFailed || p.Batch() { + t.Fatalf("the emptied batch is %s", p.State) + } +} + +// The stopped walk, the batch and the merges moved are one act (ADR 0299 decision 7): a plan written by somebody +// else since it was read refuses all of it, and no merge moves, no plan is written. +func TestSavingPlansAndMovingMergesIsOneAct(t *testing.T) { + open := windowed(t) + ctx := t.Context() + hear(t, open, catalogueMerge("cat1aaaaaaaa", "app", t0), t0.Add(time.Second)) + _, bs := walks(t, open) + stale := bs[0] + moved := stale + moved.Note = "written by somebody else" + if err := open.inventory.SavePlan(ctx, &moved); err != nil { + t.Fatal(err) + } + fresh := inventory.Plan{ID: "plan-fresh", Repository: "novox/mesh-catalog", Branch: "main", Commit: "x", Created: t0, + State: inventory.PlanAssembling, Tiers: [][]string{}, Modules: map[string]*inventory.PlanModule{}} + err := open.inventory.SavePlansMoving(ctx, []*inventory.Plan{&fresh, &stale}, + []inventory.MergeMove{{Repository: "novox/mesh-catalog", Commit: "cat1aaaaaaaa", Plan: "plan-fresh"}}) + if !errors.Is(err, inventory.ErrPlanMoved) { + t.Fatalf("a stale plan in the act: %v", err) + } + if _, err := open.inventory.PlanByID(ctx, "plan-fresh"); err == nil { + t.Fatal("the new plan was written though the act was refused") + } + if m, _, _ := open.inventory.MergeOf(ctx, "novox/mesh-catalog", "cat1aaaaaaaa"); m.Plan != stale.ID { + t.Fatalf("the merge moved to %q though the act was refused", m.Plan) + } +} diff --git a/cmd/mesh-controller/plan_retry.go b/cmd/mesh-controller/plan_retry.go index df28926e..a41acb69 100644 --- a/cmd/mesh-controller/plan_retry.go +++ b/cmd/mesh-controller/plan_retry.go @@ -278,6 +278,13 @@ func retryPlan(ctx context.Context, open *stores, id string) (string, error) { plans = append(plans, recent...) // **A failed walk whose earlier merges are walked alone is not retried** (novox/hq ADR 0276): the search for // the merge that brought the failure answers them now, and a retried walk would name them twice. + // **A walk stopped without some merges is not retried** (novox/hq ADR 0299): the merges it left out were stopped, + // and the others are another walk's now. + if p.Delivery != nil && (p.Delivery.WalkedAgainBy != "" || len(p.Delivery.LeftOut) > 0) { + return "", fmt.Errorf("%s was stopped without %s: those merges were stopped, and %s walks the others; a "+ + "newer merge, or `rebuild `, builds again", p.ID, mergesLeftOutWords(p.Delivery.LeftOut), + orWords(p.Delivery.WalkedAgainBy, "nothing")) + } if p.Delivery != nil && len(p.Delivery.Merges) > 0 { kept, err := inv.MergesOf(ctx, p.ID) if err != nil { diff --git a/internal/catalogue/verbs.go b/internal/catalogue/verbs.go index 965f06bf..40bba5f7 100644 --- a/internal/catalogue/verbs.go +++ b/internal/catalogue/verbs.go @@ -161,9 +161,11 @@ var ControllerVerbs = []Verb{ []string{"plan"})}, {Name: "delivery-stop", Description: "End a walk on its delivery's word (novox/hq ADR 0239): failed, said as stopped " + "by whom and why; what it asked still builds and registers, nothing further is asked or sent. Given merges to " + - "leave out, every other merge the walk answered is walked again in a fresh batch without them and without each " + - "later merge of their repository, which contains one and is left out too, named (novox/hq ADR 0299); the " + - "answer names the batch, the merges walked again and the merges left out.", + "leave out, every other merge the walk answered is walked again — in the open batch of its kind, or a new one — " + + "without them, without each later merge of their repository, which contains one, and without each merge moving " + + "a module one moves, each left out too and named (novox/hq ADR 0299); a batch not yet cut drops the merges. The " + + "answer names the batch, the merges walked again, the merges left out, the modules the stopped walk sent that " + + "stay on their machines, and the merges elsewhere that will still deliver a stopped change.", Input: schema(map[string]string{"plan": "the walk's id", "why": "why", "by": "who stopped the delivery", "without": "merges to leave out, @, comma-separated"}, []string{"plan", "why"})}, diff --git a/internal/inventory/plans.go b/internal/inventory/plans.go index cbc20f3f..70304f78 100644 --- a/internal/inventory/plans.go +++ b/internal/inventory/plans.go @@ -111,6 +111,12 @@ type PlanBatch struct { // a merge never shares a batch with one that waits for mesh-delivery's (ADR 0276, decided during the // build, 2026-10-10). Own bool `json:"own,omitempty"` + // Closed says the window was closed by a stop that walks a stopped walk's merges again in this batch: they + // were heard before, and an older commit never waits behind a newer one (novox/hq ADR 0299). Withheld are + // the modules a stopped merge moves: built at the branch, which holds that merge, they would deliver it, so + // the batch's walk does not build them. + Closed bool `json:"closed,omitempty"` + Withheld []string `json:"withheld,omitempty"` } // OwnPath says the record is a batch of merges on the controller's own path. @@ -192,6 +198,9 @@ type PlanDelivery struct { // ends stopped with the walk, one of a merge walked again follows that batch's walk. WalkedAgainBy string `json:"walked_again_by,omitempty"` LeftOut []PlanLeftOut `json:"left_out,omitempty"` + // Kept are the modules the stopped walk already sent and nothing walks again: the machines named keep running + // the stopped walk's build (novox/hq ADR 0299). + Kept []PlanKept `json:"kept,omitempty"` // Batch is the window of a batch; nil once it is cut into a walk. Batch *PlanBatch `json:"batch,omitempty"` // Alone says the walk walks one merge on its own commit, after a failed walk carried it in a later one: @@ -205,8 +214,18 @@ type PlanDelivery struct { type PlanLeftOut struct { Repository string `json:"repository"` Commit string `json:"commit"` - // Contains is the stopped merge a later merge contains; empty for a merge stopped itself. + // Contains is the stopped merge a later merge contains, or whose modules it shares; empty for a merge + // stopped itself. Contains string `json:"contains,omitempty"` + // Shares are the modules it moves that a stopped merge moves too: built at the branch, which holds the stopped + // merge, they would deliver it. + Shares []string `json:"shares,omitempty"` +} + +// PlanKept is a module a stopped walk sent and nothing walks again, and the machines it reached. +type PlanKept struct { + Module string `json:"module"` + Machines []string `json:"machines"` } // Waiting is whether the walk waits for its delivery's word. @@ -400,39 +419,28 @@ func (p Plan) Open() bool { return p.State == PlanBuilding || p.State == PlanRol // with ErrPlanMoved otherwise; and only by a process that may act (ActsUnder), whose epoch it records. // On success p's revision and epoch are the ones written, so the caller may save it again. func (i *Inventory) SavePlan(ctx context.Context, p *Plan) error { + return i.SavePlansMoving(ctx, []*Plan{p}, nil) +} + +// MergeMove is a merge given to the plan that answers it now (novox/hq ADR 0276). +type MergeMove struct { + Repository, Commit, Plan string + Alone bool +} + +// SavePlansMoving writes plans, as SavePlan does each, and gives merges to the plans that answer them, all in one +// transaction: every write and move, or none (novox/hq ADR 0299 decision 7 — a stop that walks merges again never +// 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 { epoch, err := i.actingEpoch(ctx) if err != nil { - return fmt.Errorf("the plan for %s %s is not written: %w", p.Repository, p.Commit, err) - } - tiers, err := json.Marshal(p.Tiers) - if err != nil { + if len(plans) > 0 { + return fmt.Errorf("the plan for %s %s is not written: %w", plans[0].Repository, plans[0].Commit, err) + } return err } - modules, err := json.Marshal(p.Modules) - if err != nil { - return err - } - var release, delivery, commits, times []byte - if p.Times != nil { - if times, err = json.Marshal(p.Times); err != nil { - return err - } - } - if len(p.Commits) > 0 { - if commits, err = json.Marshal(p.Commits); err != nil { - return err - } - } - if p.Release != nil { - if release, err = json.Marshal(p.Release); err != nil { - return err - } - } - if p.Delivery != nil { - if delivery, err = json.Marshal(p.Delivery); err != nil { - return err - } - } // **And how long the tier it left took** (novox/hq to-be 45 Phase 0): measured here, where the // plan moves, in the same transaction as the move, so no save can move a tier unmeasured or // measure one twice. @@ -441,9 +449,77 @@ func (i *Inventory) SavePlan(ctx context.Context, p *Plan) error { return err } defer func() { _ = tx.Rollback(ctx) }() + type written struct { + revision int64 + entered time.Time + } + done := make([]written, len(plans)) + for n, p := range plans { + revision, entered, err := writePlan(ctx, tx, p, epoch) + if err != nil { + return err + } + done[n] = written{revision, entered} + } + for _, m := range moves { + if _, err := tx.Exec(ctx, + `update batched_merge set plan_id = nullif($3, ''), alone = $4 where repository = lower($1) and commit_hash = $2`, + m.Repository, m.Commit, m.Plan, m.Alone); err != nil { + return err + } + } + if err := tx.Commit(ctx); err != nil { + return err + } + for n, p := range plans { + p.Revision, p.TierEntered = done[n].revision, done[n].entered + if epoch != nil { + p.Epoch = uint64(*epoch) + } else { + p.Epoch = 0 + } + if PlanSaved != nil { + PlanSaved(*p) + } + } + return nil +} + +// writePlan writes one plan within a transaction, by compare-and-set on its revision: its new revision, and when +// it entered its tier. +func writePlan(ctx context.Context, tx pgx.Tx, p *Plan, epoch *int64) (int64, time.Time, error) { + tiers, err := json.Marshal(p.Tiers) + if err != nil { + return 0, time.Time{}, err + } + modules, err := json.Marshal(p.Modules) + if err != nil { + return 0, time.Time{}, err + } + var release, delivery, commits, times []byte + if p.Times != nil { + if times, err = json.Marshal(p.Times); err != nil { + return 0, time.Time{}, err + } + } + if len(p.Commits) > 0 { + if commits, err = json.Marshal(p.Commits); err != nil { + return 0, time.Time{}, err + } + } + if p.Release != nil { + if release, err = json.Marshal(p.Release); err != nil { + return 0, time.Time{}, err + } + } + if p.Delivery != nil { + if delivery, err = json.Marshal(p.Delivery); err != nil { + return 0, time.Time{}, err + } + } entered, err := planTierLeft(ctx, tx, *p, time.Now()) if err != nil { - return err + return 0, time.Time{}, err } var revision int64 err = tx.QueryRow(ctx, @@ -464,24 +540,9 @@ func (i *Inventory) SavePlan(ctx context.Context, p *Plan) error { // The row is there and at another revision — moved since this was read, or there already // when this one is new: either way not this writer's to overwrite. (A plan saved before plans // had revisions is at zero, and its first save here is from a read at zero.) - return fmt.Errorf("the plan for %s %s (%s) is not written: %w", p.Repository, short(p.Commit), p.ID, ErrPlanMoved) + return 0, time.Time{}, fmt.Errorf("the plan for %s %s (%s) is not written: %w", p.Repository, short(p.Commit), p.ID, ErrPlanMoved) } - if err != nil { - return err - } - if err := tx.Commit(ctx); err != nil { - return err - } - p.Revision, p.TierEntered = revision, entered - if epoch != nil { - p.Epoch = uint64(*epoch) - } else { - p.Epoch = 0 - } - if PlanSaved != nil { - PlanSaved(*p) - } - return nil + return revision, entered, err } // short is a commit as a person reads it. diff --git a/merge-check.sh b/merge-check.sh index 296fe271..775baa06 100755 --- a/merge-check.sh +++ b/merge-check.sh @@ -32,3 +32,18 @@ else echo "NOT RACE-CHECKED: the toolchain holds no C compiler; the suite runs without the race detector" CGO_ENABLED=0 go test -count=1 -timeout 30m ./... fi + +# **The store tests of novox/hq ADR 0299 ran** (review of #232): a test that needs the store skips without one, +# and a skip passes unseen. With the store this check is given, each must have run and passed, said by name; +# without one, that is said. +adr0299='^(TestAStopWithoutAMergeWalksTheOthersAgain|TestAStopWithoutAMergeEndsTheMergesThatContainIt|TestAStopWalksTheOthersAgainInTheOpenBatch|TestAStopNamesWhatStaysOnTheMachinesAndWhatStillCarriesIt|TestAStopWithoutAMergeLeavesItOutOfABatch|TestSavingPlansAndMovingMergesIsOneAct)$' +if [ -n "${MESH_TEST_POSTGRES:-}" ]; then + ran=$(CGO_ENABLED=0 go test -count=1 -v -run "$adr0299" ./cmd/mesh-controller 2>&1 | grep -E '^--- (PASS|FAIL|SKIP)' || true) + printf 'ADR 0299 store tests:\n%s\n' "$ran" + if [ "$(printf '%s\n' "$ran" | grep -c '^--- PASS')" -ne 6 ]; then + echo "ADR 0299's store tests did not all run and pass against the store" + exit 1 + fi +else + echo "NOT STORE-CHECKED: no MESH_TEST_POSTGRES, so ADR 0299's store tests skipped" +fi From 932203df205026f6a35184ee8b51c873d72c644f Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 11 Oct 2026 20:23:14 +0200 Subject: [PATCH 3/5] Leave the store tests' proof to the check of its own (#233) --- merge-check.sh | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/merge-check.sh b/merge-check.sh index 775baa06..296fe271 100755 --- a/merge-check.sh +++ b/merge-check.sh @@ -32,18 +32,3 @@ else echo "NOT RACE-CHECKED: the toolchain holds no C compiler; the suite runs without the race detector" CGO_ENABLED=0 go test -count=1 -timeout 30m ./... fi - -# **The store tests of novox/hq ADR 0299 ran** (review of #232): a test that needs the store skips without one, -# and a skip passes unseen. With the store this check is given, each must have run and passed, said by name; -# without one, that is said. -adr0299='^(TestAStopWithoutAMergeWalksTheOthersAgain|TestAStopWithoutAMergeEndsTheMergesThatContainIt|TestAStopWalksTheOthersAgainInTheOpenBatch|TestAStopNamesWhatStaysOnTheMachinesAndWhatStillCarriesIt|TestAStopWithoutAMergeLeavesItOutOfABatch|TestSavingPlansAndMovingMergesIsOneAct)$' -if [ -n "${MESH_TEST_POSTGRES:-}" ]; then - ran=$(CGO_ENABLED=0 go test -count=1 -v -run "$adr0299" ./cmd/mesh-controller 2>&1 | grep -E '^--- (PASS|FAIL|SKIP)' || true) - printf 'ADR 0299 store tests:\n%s\n' "$ran" - if [ "$(printf '%s\n' "$ran" | grep -c '^--- PASS')" -ne 6 ]; then - echo "ADR 0299's store tests did not all run and pass against the store" - exit 1 - fi -else - echo "NOT STORE-CHECKED: no MESH_TEST_POSTGRES, so ADR 0299's store tests skipped" -fi From 201bd2827841f84943c4d918ef003d012bd2147f Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 11 Oct 2026 20:25:20 +0200 Subject: [PATCH 4/5] Keep a walk's withheld modules on its record, name every machine a stopped send reached, and list the stop's store tests (review of #232) --- cmd/mesh-controller/batches.go | 3 +++ cmd/mesh-controller/delivery.go | 19 ++++++++++++++++--- cmd/mesh-controller/issue459_test.go | 19 +++++++++++++++++++ internal/inventory/plans.go | 3 +++ testdata/store-tests | 6 ++++++ 5 files changed, 47 insertions(+), 3 deletions(-) diff --git a/cmd/mesh-controller/batches.go b/cmd/mesh-controller/batches.go index a0db5821..7b4704a7 100644 --- a/cmd/mesh-controller/batches.go +++ b/cmd/mesh-controller/batches.go @@ -768,6 +768,7 @@ func planBatch(ctx context.Context, open *stores, batch *inventory.Plan, carry [ } sort.Strings(also) ordered := groupOrderEdges(members, entries, read, edges) + var withheld []string plan := planOfMoves(moved, append(append([]inventory.Edge{}, edges...), ordered...)) // **Withheld, a stopped merge's modules** (novox/hq ADR 0299): a module is built at its branch, which holds the // merge a stop left out of the walk these merges were walked again from — built here only as a dependent, it @@ -775,6 +776,7 @@ func planBatch(ctx context.Context, open *stores, batch *inventory.Plan, carry [ // and the stop named it. if batch.Delivery != nil && batch.Delivery.Batch != nil { if out := withhold(&plan, batch.Delivery.Batch.Withheld, moved); len(out) > 0 { + withheld = out fmt.Printf(" withheld, a stopped merge's modules built only as dependents: %s\n", strings.Join(out, ", ")) } } @@ -788,6 +790,7 @@ func planBatch(ctx context.Context, open *stores, batch *inventory.Plan, carry [ } plan.Delivery.Merges = named plan.Delivery.Alone = batch.Delivery != nil && batch.Delivery.Alone + plan.Delivery.Withheld = withheld // **Its moments and its class** (novox/hq ADR 0282 decision 6): measured, never acted on. plan.Times = walkTimesAtCut(*batch, plan, entries, now) if len(moved) == 0 { diff --git a/cmd/mesh-controller/delivery.go b/cmd/mesh-controller/delivery.go index f19ee10b..56e4a077 100644 --- a/cmd/mesh-controller/delivery.go +++ b/cmd/mesh-controller/delivery.go @@ -191,7 +191,15 @@ func stopWalkWithout(ctx context.Context, inv *inventory.Inventory, id, by, why if batch { return p, said, stopInBatch(ctx, inv, &p, merges, again, &said, stopped, now) } - said.Kept = keptOf(p, movesOf(again)) + entries, err := inv.Catalogued(ctx) + if err != nil { + return p, said, err + } + running := map[string][]string{} + for _, e := range entries { + running[e.Manifest.Module] = e.On + } + said.Kept = keptOf(p, movesOf(again), running) stopped(&p) p.Delivery.Kept = said.Kept if len(again) == 0 { @@ -413,8 +421,10 @@ func movesOf(merges []inventory.BatchedMerge) []string { } // keptOf are the modules a stopped walk sent that no merge walked again moves, and the machines each reached: -// those keep running the stopped walk's build. -func keptOf(p inventory.Plan, walkedAgain []string) []inventory.PlanKept { +// those keep running the stopped walk's build. A module sent to its first machines reached those, and the rest it +// was sent to after them; one sent with no first machine (its policy sends it to all together, or a later tier is +// built by it) reached every machine running it. +func keptOf(p inventory.Plan, walkedAgain []string, running map[string][]string) []inventory.PlanKept { var out []inventory.PlanKept for name, m := range p.Modules { if m == nil || slices.Contains(walkedAgain, name) { @@ -424,6 +434,9 @@ func keptOf(p inventory.Plan, walkedAgain []string) []inventory.PlanKept { if m.FirstAt != nil || m.SentAt != nil { machines = union(machines, m.First) } + if m.SentAt != nil && len(m.First) == 0 { + machines = union(machines, running[name]) + } if m.Gate != nil && m.Gate.Since != nil { machines = union(machines, m.Gate.Machines) } diff --git a/cmd/mesh-controller/issue459_test.go b/cmd/mesh-controller/issue459_test.go index b57accc3..0aa17fbc 100644 --- a/cmd/mesh-controller/issue459_test.go +++ b/cmd/mesh-controller/issue459_test.go @@ -391,3 +391,22 @@ func TestSavingPlansAndMovingMergesIsOneAct(t *testing.T) { t.Fatalf("the merge moved to %q though the act was refused", m.Plan) } } + +// What a stopped walk leaves on the machines names every machine its sends reached (review of #232): a module sent +// with no first machine reached every machine running it; one sent first reached those, and the rest it was sent +// to; a module a merge walked again moves is sent again, and is not named. +func TestKeptNamesEveryMachineASendReached(t *testing.T) { + at := t0 + p := inventory.Plan{Modules: map[string]*inventory.PlanModule{ + "together": {SentAt: &at}, + "first": {First: []string{"anchor"}, FirstAt: &at, Rest: map[string]inventory.SentDeclaration{"laptop": {}}}, + "again": {SentAt: &at}, + "unsent": {}, + }} + got := keptOf(p, []string{"again"}, map[string][]string{"together": {"laptop", "anchor"}, "again": {"anchor"}}) + want := []inventory.PlanKept{{Module: "first", Machines: []string{"anchor", "laptop"}}, + {Module: "together", Machines: []string{"anchor", "laptop"}}} + if !reflect.DeepEqual(got, want) { + t.Fatalf("kept %+v; want %+v", got, want) + } +} diff --git a/internal/inventory/plans.go b/internal/inventory/plans.go index c5b43a8b..b7d5ad24 100644 --- a/internal/inventory/plans.go +++ b/internal/inventory/plans.go @@ -201,6 +201,9 @@ type PlanDelivery struct { // Kept are the modules the stopped walk already sent and nothing walks again: the machines named keep running // the stopped walk's build (novox/hq ADR 0299). Kept []PlanKept `json:"kept,omitempty"` + // Withheld are, on a walk cut from a batch a stop walked merges again in, the modules it did not build: a + // stopped merge moves them, and built only as dependents at the branch they would deliver it (ADR 0299). + Withheld []string `json:"withheld,omitempty"` // Batch is the window of a batch; nil once it is cut into a walk. Batch *PlanBatch `json:"batch,omitempty"` // Alone says the walk walks one merge on its own commit, after a failed walk carried it in a later one: diff --git a/testdata/store-tests b/testdata/store-tests index 1a66198b..17c6cc1e 100644 --- a/testdata/store-tests +++ b/testdata/store-tests @@ -1,3 +1,9 @@ # The tests that need the throwaway store and must have run against it (novox/hq issue 459): merge-check.sh runs # each one named here, by package, and fails unless every one passed — a test that needs the store skips without # one, and a skip passes unseen. One per line: . Lines starting with # are said nothing of. +./cmd/mesh-controller TestAStopWithoutAMergeWalksTheOthersAgain +./cmd/mesh-controller TestAStopWithoutAMergeEndsTheMergesThatContainIt +./cmd/mesh-controller TestAStopWalksTheOthersAgainInTheOpenBatch +./cmd/mesh-controller TestAStopNamesWhatStaysOnTheMachinesAndWhatStillCarriesIt +./cmd/mesh-controller TestAStopWithoutAMergeLeavesItOutOfABatch +./cmd/mesh-controller TestSavingPlansAndMovingMergesIsOneAct From bc4662d656efc631b8446e89711e2e76a7732a5a Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 11 Oct 2026 21:06:44 +0200 Subject: [PATCH 5/5] End the line a walk's ask leaves open as each ADR 0299 store test ends, so its pass is read (issue 459) A walk's ask prints " tier N: " and the build ask the rest; the tests' fake ask prints nothing, so go test -v put three tests' --- PASS after it and the store-tests step read them as not run. --- cmd/mesh-controller/issue459_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/cmd/mesh-controller/issue459_test.go b/cmd/mesh-controller/issue459_test.go index 0aa17fbc..37f1782e 100644 --- a/cmd/mesh-controller/issue459_test.go +++ b/cmd/mesh-controller/issue459_test.go @@ -3,6 +3,7 @@ package main import ( "encoding/json" "errors" + "fmt" "reflect" "slices" "strings" @@ -17,6 +18,14 @@ import ( // to leave out: the walk is ended, and every other merge it answered is walked again in a fresh batch without them // — the stopped merge, and each later merge of its repository's branch, which contains it and ends with it. +// endsItsLine ends, as the test ends, a line the controller's output left open (a walk's ask prints " tier N: " and +// the asked build the rest, which the tests' fake ask never prints): `go test -v` would put the test's "--- PASS" +// after it, and the store-tests step of merge-check.sh, which reads that marker at a line's start, would say the +// test never ran. +func endsItsLine(t *testing.T) { + t.Cleanup(func() { fmt.Println() }) +} + // leaved is a kept merge of a repository, made at a moment. func leaved(repository, commit string, made time.Time) inventory.BatchedMerge { return inventory.BatchedMerge{Repository: repository, Branch: "main", Commit: commit, Merged: made, Heard: made} @@ -114,6 +123,7 @@ func TestDeliveryStopTakesTheMergesToLeaveOut(t *testing.T) { // that builds the other repository's module alone. func TestAStopWithoutAMergeWalksTheOthersAgain(t *testing.T) { open := windowed(t) + endsItsLine(t) asksRecorded(t) ctx := t.Context() cat := catalogueMerge("cat1aaaaaaaa", "app", t0) @@ -171,6 +181,7 @@ func TestAStopWithoutAMergeWalksTheOthersAgain(t *testing.T) { // ended. A merge the walk does not answer is refused, and nothing ends. func TestAStopWithoutAMergeEndsTheMergesThatContainIt(t *testing.T) { open := windowed(t) + endsItsLine(t) asksRecorded(t) ctx := t.Context() first := catalogueMerge("cat1aaaaaaaa", "app", t0) @@ -227,6 +238,7 @@ func TestAStopWithoutAMergeEndsTheMergesThatContainIt(t *testing.T) { // (review of #232): its window closes, and its walk is at the newer commit, answering both. func TestAStopWalksTheOthersAgainInTheOpenBatch(t *testing.T) { open := windowed(t) + endsItsLine(t) asksRecorded(t) ctx := t.Context() cat := catalogueMerge("cat1aaaaaaaa", "app", t0) @@ -278,6 +290,7 @@ func TestAStopWalksTheOthersAgainInTheOpenBatch(t *testing.T) { // that walked its merges again is refused a retry, saying why. func TestAStopNamesWhatStaysOnTheMachinesAndWhatStillCarriesIt(t *testing.T) { open := windowed(t) + endsItsLine(t) asksRecorded(t) ctx := t.Context() cat := catalogueMerge("cat1aaaaaaaa", "app", t0) @@ -322,6 +335,7 @@ func TestAStopNamesWhatStaysOnTheMachinesAndWhatStillCarriesIt(t *testing.T) { // with the others and withholds the stopped merge's modules; with every merge left out, the batch is stopped. func TestAStopWithoutAMergeLeavesItOutOfABatch(t *testing.T) { open := windowed(t) + endsItsLine(t) asksRecorded(t) ctx := t.Context() cat := catalogueMerge("cat1aaaaaaaa", "app", t0) @@ -368,6 +382,7 @@ func TestAStopWithoutAMergeLeavesItOutOfABatch(t *testing.T) { // else since it was read refuses all of it, and no merge moves, no plan is written. func TestSavingPlansAndMovingMergesIsOneAct(t *testing.T) { open := windowed(t) + endsItsLine(t) ctx := t.Context() hear(t, open, catalogueMerge("cat1aaaaaaaa", "app", t0), t0.Add(time.Second)) _, bs := walks(t, open)