From 3d192a4857f33d99db47b541403b3af80edb6a15 Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 17 Oct 2026 13:30:27 +0200 Subject: [PATCH] issue 974: every plan is written through the one hook that says plan-moved, proven by reading the sources and by the moves a merge and a cut make --- .../plan_moved_every_writer_test.go | 140 ++++++++++++++++++ testdata/store-tests | 1 + 2 files changed, 141 insertions(+) create mode 100644 cmd/mesh-controller/plan_moved_every_writer_test.go diff --git a/cmd/mesh-controller/plan_moved_every_writer_test.go b/cmd/mesh-controller/plan_moved_every_writer_test.go new file mode 100644 index 00000000..4be2f8aa --- /dev/null +++ b/cmd/mesh-controller/plan_moved_every_writer_test.go @@ -0,0 +1,140 @@ +package main + +import ( + "os" + "path/filepath" + "regexp" + "strings" + "sync" + "testing" + "time" + + "github.com/novox/mesh-controller/internal/inventory" +) + +// **Every plan is written through SavePlansMoving, which says it** (issue 974, novox/hq ADR 0406 §2: every move of a plan +// is said as plan-moved, so no move is found only by comparing). The one hook, inventory.PlanSaved, is called there +// after the commit; so no statement that writes the plan table may stand anywhere else, and writePlan, the one that +// does, may be called from nowhere but SavePlansMoving. Checked by reading the sources, no store needed. +func TestNoPlanIsWrittenButThroughSavePlansMoving(t *testing.T) { + write := regexp.MustCompile(`(?is)(insert\s+into|update|delete\s+from|truncate)\s+(table\s+)?release_plan\b`) + var found []string + for _, root := range []string{"../../cmd", "../../internal"} { + err := filepath.WalkDir(root, func(path string, d os.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() || !strings.HasSuffix(path, ".go") || strings.HasSuffix(path, "_test.go") { + return nil + } + raw, err := os.ReadFile(path) + if err != nil { + return err + } + text := string(raw) + if write.MatchString(text) && filepath.ToSlash(path) != "../../internal/inventory/plans.go" { + found = append(found, path+" writes the plan table") + } + if strings.Contains(text, "writePlan(") && filepath.ToSlash(path) != "../../internal/inventory/plans.go" { + found = append(found, path+" calls writePlan") + } + return nil + }) + if err != nil { + t.Fatal(err) + } + } + if len(found) > 0 { + t.Fatalf("a plan is written without plan-moved being said:\n%s", strings.Join(found, "\n")) + } + raw, err := os.ReadFile("../../internal/inventory/plans.go") + if err != nil { + t.Fatal(err) + } + text := string(raw) + if n := len(write.FindAllString(text, -1)); n != 1 { + t.Errorf("plans.go has %d statements that write the plan table; want the one in writePlan", n) + } + calls := regexp.MustCompile(`writePlan\(ctx, tx`).FindAllStringIndex(text, -1) + save := strings.Index(text, "func (i *Inventory) SavePlansMoving") + next := strings.Index(text[save:], "\nfunc ") + if len(calls) != 1 || calls[0][0] < save || calls[0][0] > save+next { + t.Errorf("writePlan is called %d times, and not only from SavePlansMoving", len(calls)) + } + if !strings.Contains(text[save:save+next], "PlanSaved(i, *p)") { + t.Error("SavePlansMoving does not say each plan it wrote") + } +} + +// movesSaid records what the hook hears: for each plan, the revisions it was said at. +type movesSaid struct { + mu sync.Mutex + revs map[string][]int64 +} + +func (s *movesSaid) hear(_ *inventory.Inventory, p inventory.Plan) { + s.mu.Lock() + defer s.mu.Unlock() + s.revs[p.ID] = append(s.revs[p.ID], p.Revision) +} + +// sayingEverySave installs a PlanSaved that records, as the serving controller's says plan-moved. +func sayingEverySave(t *testing.T) *movesSaid { + t.Helper() + s := &movesSaid{revs: map[string][]int64{}} + was := inventory.PlanSaved + inventory.PlanSaved = s.hear + t.Cleanup(func() { inventory.PlanSaved = was }) + return s +} + +// noUnsaid fails the test for a plan whose stored revision is not one the hook heard: a move nobody said. +func (s *movesSaid) noUnsaid(t *testing.T, open *stores) { + t.Helper() + recent, err := open.inventory.RecentPlans(t.Context(), 50) + if err != nil { + t.Fatal(err) + } + s.mu.Lock() + defer s.mu.Unlock() + if len(recent) == 0 { + t.Fatal("no plan was kept") + } + for _, p := range recent { + revs := s.revs[p.ID] + if len(revs) == 0 || revs[len(revs)-1] != p.Revision { + t.Errorf("%s is kept at revision %d and was said at %v: a move nobody said", p.ID, p.Revision, revs) + } + } +} + +// Live, with the store: the moves a merge makes of a plan — the batch it opens, the walk it is cut into, the tier that +// is asked — are each said, and a look that changes nothing says nothing (novox/hq ADR 0406 §2). +func TestEveryMoveAMergeAndTheCutMakeOfAPlanIsSaid(t *testing.T) { + open := windowed(t) + everyHeadWaits(t) + asksWithPaths(t) + heard := sayingEverySave(t) + hear(t, open, catalogueMerge("553b7191claude", "app", t0), t0.Add(time.Second)) + if _, bs := walks(t, open); len(bs) != 1 { + t.Fatalf("%d batch(es) opened", len(bs)) + } + heard.noUnsaid(t, open) + cutAt(t, open, t0.Add(91*time.Second)) + if ws, bs := walks(t, open); len(ws) != 1 || len(bs) != 0 { + t.Fatalf("%d walk(s), %d batch(es) after the cut", len(ws), len(bs)) + } + heard.noUnsaid(t, open) + counted := 0 + for _, r := range heard.revs { + counted += len(r) + } + cutAt(t, open, t0.Add(92*time.Second)) + again := 0 + for _, r := range heard.revs { + again += len(r) + } + if again != counted { + t.Errorf("a look that changed nothing said %d more move(s)", again-counted) + } +} diff --git a/testdata/store-tests b/testdata/store-tests index d2d9f485..3d3dff3a 100644 --- a/testdata/store-tests +++ b/testdata/store-tests @@ -133,3 +133,4 @@ ./cmd/mesh-controller TestAnOwnPathBatchNeverWalksBeside ./cmd/mesh-controller TestMergesHeardWhileAnotherHolderHasThePlansAreRecordedAndCarried ./cmd/mesh-controller TestABatchWindowClosesByTheExpiryOfItsKey +./cmd/mesh-controller TestEveryMoveAMergeAndTheCutMakeOfAPlanIsSaid