From af4eaefb14cb9d03ad01cd4dc44245ecbcf1da55 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 22 Oct 2026 18:09:56 +0200 Subject: [PATCH] Read the controller's build records only for the builder's own input and to fill a key's missing directory once, so a key written before paths gets one with no merge (issue 1851 I3, issue 1614) Both reads go in R4g with build-agent. A path a key names is never overwritten, and no fingerprint changes. --- .../mesh-delivery-indexer/integration_test.go | 10 +- .../internal/index/builds.go | 131 +++++------------- .../internal/index/live_builds_test.go | 16 +-- .../internal/index/paths_backfill_test.go | 67 +++++++++ .../internal/index/toolchains.go | 43 ++++++ 5 files changed, 159 insertions(+), 108 deletions(-) create mode 100644 modules/mesh-delivery-indexer/internal/index/paths_backfill_test.go diff --git a/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/integration_test.go b/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/integration_test.go index 2d49b96..e65d5fd 100644 --- a/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/integration_test.go +++ b/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/integration_test.go @@ -183,8 +183,14 @@ func startIndexer(t *testing.T, put func(context.Context, jetstream.KeyValue), b // TestIndexerWritesModulesAtAMerge: a module's record and its newest pointer are in mesh-controller_builds; a merged // message on FORGE makes modules. within 5 s, the fingerprint of the merge's tree and the recorded inputs. -func TestIndexerWritesModulesAtAMerge(t *testing.T) { // 1239/AC9; I1: the modules are the merged message's, no build record names them (issue 1851) - js, ctx, _, modules := startIndexer(t, func(context.Context, jetstream.KeyValue) {}) +func TestIndexerWritesModulesAtAMerge(t *testing.T) { // 1239/AC9; I1, I3: the modules are the merged message's, no build record names them (issue 1851) + // The bucket holds only build-agent's record, the one the indexer reads until R4g (the builder's own input). + js, ctx, _, modules := startIndexer(t, func(ctx context.Context, builds jetstream.KeyValue) { + rec := `{"id":"build-1","repository":"novox/mesh-controller","ref":"main","module":"build-agent","commit":"7a74cee83f88e6585a5511c3cc53bdf0f5303d20","path":"modules/build-agent","source-fingerprint":"src1:e846efe73716b78e80860baaac96153de2cb76551325753838ca245d2160a3d7","at":"2026-10-19T09:38:00Z"}` + if _, err := builds.Put(ctx, delivery.BuildKey("build-agent", "build-1"), []byte(rec)); err != nil { + t.Fatal(err) + } + }) const merge = "89abcdef0123456789abcdef0123456789abcdef" data, err := delivery.EncodeForge(delivery.Forge{Kind: delivery.ForgeMerged, Repository: "novox/mesh-delivery", Branch: "feat/x", Number: 12, Head: "0123456789abcdef0123456789abcdef01234567", Merge: merge, Root: "r0", Trees: map[string]string{"modules/a": "newtree"}, diff --git a/modules/mesh-delivery-indexer/internal/index/builds.go b/modules/mesh-delivery-indexer/internal/index/builds.go index 72eec2a..5dfd681 100644 --- a/modules/mesh-delivery-indexer/internal/index/builds.go +++ b/modules/mesh-delivery-indexer/internal/index/builds.go @@ -1,31 +1,24 @@ -// Package index keeps the candidate groups (novox/hq ADR 0437 ยง5) the forge bridge's pull messages make. package index import ( "encoding/json" - "path" - "sort" "strings" "sync" "git.novox.be/novox/mesh-sdk/go/delivery" ) -// Builds holds, from the controller's builds bucket (mesh-controller_builds), which module a directory of a -// repository is: the module of the newest build record naming that repository and path (BuildRecord.Path). Safe for use -// from a watch and from the message handler at once. +// Builds holds the controller's build records (mesh-controller_builds) for the two things the indexer still reads from +// them until release R4g, when build-agent goes (the lead's decision of R4e, 2026-10-22): +// - the Builder input of the builder's own module, which build-agent builds (TrunkOf; novox/hq ADR 0453 clause 5, +// issue 1727); +// - the directory of a module whose modules key has none (PathOf; issue 1614), filled once and changing no fingerprint. +// +// It lists no module and maps no directory to a module: the indexer's modules are those its own messages and bucket name +// (ADR 0501 clause 6, issue 1851). Safe for use from a watch and from the message handler at once. type Builds struct { mu sync.Mutex records map[string]delivery.BuildRecord // by bucket key - byDir map[string]delivery.BuildRecord // by repository and path; nil when stale - mods map[string]*moduleBuilds // by module; nil when stale -} - -// moduleBuilds is what the builds say of one module: its newest record of any kind, and its newest successful trunk -// build that recorded its inputs. -type moduleBuilds struct { - any delivery.BuildRecord - trunk *delivery.BuildRecord } // NewBuilds is an empty Builds. @@ -44,101 +37,19 @@ func (b *Builds) Put(key string, value []byte) { b.mu.Lock() defer b.mu.Unlock() b.records[key] = r - b.byDir, b.mods = nil, nil } // Delete takes a key out. func (b *Builds) Delete(key string) { b.mu.Lock() defer b.mu.Unlock() - if _, ok := b.records[key]; ok { - delete(b.records, key) - b.byDir, b.mods = nil, nil - } -} - -// dirKey names a directory of a repository whichever way the repository is spelled: a build record names it by its clone -// URL, a forge message by "owner/name" (issue 1281). -func dirKey(repository, dir string) string { - return strings.ToLower(delivery.RepositoryName(repository)) + "\x00" + strings.Trim(dir, "/") -} - -// Module is the module a build record of the repository names the directory as its path, if one does. -func (b *Builds) Module(repository, dir string) (string, bool) { - b.mu.Lock() - defer b.mu.Unlock() - if b.byDir == nil { - b.byDir = map[string]delivery.BuildRecord{} - for _, r := range b.records { - k := dirKey(r.Repository, r.Path) - if old, ok := b.byDir[k]; !ok || r.At.After(old.At) || (r.At.Equal(old.At) && r.Module < old.Module) { - b.byDir[k] = r - } - } - } - r, ok := b.byDir[dirKey(repository, dir)] - return r.Module, ok -} - -// ModuleOf is the module of a changed directory: the one a build record of the repository names it the path of; a -// directory no build record names is the directory's last element (the catalogue's rule for nearly every manifest). -func (b *Builds) ModuleOf(repository, dir string) string { - if m, ok := b.Module(repository, dir); ok { - return m - } - return path.Base(strings.Trim(dir, "/")) -} - -// Known is a module the builds know of in a repository: its directory there and its newest successful trunk build -// (delivery.BuildRecord.IsTrunkBuild) if any. The newest key of a module is any state and ref, so it is not what -// finds this (mesh-sdk go/delivery/buildrecord.go NewestKey); the records are. -type Known struct { - Module string - Repository string - Path string - Trunk *delivery.BuildRecord + delete(b.records, key) } func newer(a, b delivery.BuildRecord) bool { return a.At.After(b.At) || (a.At.Equal(b.At) && a.ID > b.ID) } -// Modules are the modules built from a repository, by name. A module whose newest successful trunk build is of the -// repository belongs to it with that record's path; a module with no such build belongs to the repository of its newest -// record of any kind. -func (b *Builds) Modules(repository string) []Known { - b.mu.Lock() - defer b.mu.Unlock() - if b.mods == nil { - b.mods = map[string]*moduleBuilds{} - for _, r := range b.records { - m := b.mods[r.Module] - if m == nil { - m = &moduleBuilds{any: r} - b.mods[r.Module] = m - } else if newer(r, m.any) { - m.any = r - } - if r.IsTrunkBuild() && (m.trunk == nil || newer(r, *m.trunk)) { - c := r - m.trunk = &c - } - } - } - var out []Known - for name, m := range b.mods { - k := Known{Module: name, Repository: m.any.Repository, Path: strings.Trim(m.any.Path, "/"), Trunk: m.trunk} - if m.trunk != nil { - k.Repository, k.Path = m.trunk.Repository, strings.Trim(m.trunk.Path, "/") - } - if delivery.SameRepository(k.Repository, repository) { - out = append(out, k) - } - } - sort.Slice(out, func(i, j int) bool { return out[i].Module < out[j].Module }) - return out -} - // TrunkOf is the newest successful trunk build record of a module, in whatever repository it was built from; nil when // there is none (issue 1727). func (b *Builds) TrunkOf(module string) *delivery.BuildRecord { @@ -153,3 +64,27 @@ func (b *Builds) TrunkOf(module string) *delivery.BuildRecord { } return best } + +// PathOf is the directory of a module of the repository as its newest build record of that repository names it, its +// newest successful trunk build's first ("." for the repository's root), and false when no record of the repository +// names the module (issue 1614). +func (b *Builds) PathOf(module, repository string) (string, bool) { + var best *delivery.BuildRecord + trunk := false + b.mu.Lock() + defer b.mu.Unlock() + for _, r := range b.records { + if r.Module != module || !delivery.SameRepository(r.Repository, repository) { + continue + } + t := r.IsTrunkBuild() + if best == nil || (t && !trunk) || (t == trunk && newer(r, *best)) { + c := r + best, trunk = &c, t + } + } + if best == nil { + return "", false + } + return pathOf(strings.Trim(best.Path, "/")), true +} diff --git a/modules/mesh-delivery-indexer/internal/index/live_builds_test.go b/modules/mesh-delivery-indexer/internal/index/live_builds_test.go index a196a54..7aacda2 100644 --- a/modules/mesh-delivery-indexer/internal/index/live_builds_test.go +++ b/modules/mesh-delivery-indexer/internal/index/live_builds_test.go @@ -50,17 +50,17 @@ func liveIndexer(t *testing.T) (*Indexer, *memStore) { return ix, mods } -func TestChangedDirectoryMapsToTheModuleItsLiveRecordNamesByPath(t *testing.T) { // 1281/AC4 +func TestTheLiveRecordsGiveTheDirectoryOfAModuleOfTheirRepository(t *testing.T) { // 1614 (I3: the records name directories, never list modules) ix, _ := liveIndexer(t) - for dir, want := range map[string]string{"modules/zsh": "zsh", "modules/mesh-delivery": "mesh-delivery", "modules/mailu/": "mailu"} { - if got, ok := ix.Builds.Module("novox/mesh-catalog", dir); !ok || got != want { - t.Errorf("Module(novox/mesh-catalog, %q) = %q, %v; want %q", dir, got, ok, want) + for module, want := range map[string]string{"zsh": "modules/zsh", "mesh-delivery": "modules/mesh-delivery", "mailu": "modules/mailu"} { + if got, ok := ix.Builds.PathOf(module, "novox/mesh-catalog"); !ok || got != want { + t.Errorf("PathOf(%s, novox/mesh-catalog) = %q, %v; want %q", module, got, ok, want) } } - if got, ok := ix.Builds.Module("novox/mesh-catalog", "modules/sonarr"); ok { - t.Errorf("Module found %q for a directory of mesh-media-catalog under mesh-catalog", got) + if got, ok := ix.Builds.PathOf("sonarr", "novox/mesh-catalog"); ok { + t.Errorf("PathOf found %q for a module of mesh-media-catalog under mesh-catalog", got) } - if got := ix.Builds.ModuleOf("novox/mesh-media-catalog", "modules/sonarr"); got != "sonarr" { - t.Errorf("ModuleOf(mesh-media-catalog, modules/sonarr) = %q, want sonarr", got) + if got, ok := ix.Builds.PathOf("sonarr", "novox/mesh-media-catalog"); !ok || got != "modules/sonarr" { + t.Errorf("PathOf(sonarr, mesh-media-catalog) = %q, %v; want modules/sonarr", got, ok) } } diff --git a/modules/mesh-delivery-indexer/internal/index/paths_backfill_test.go b/modules/mesh-delivery-indexer/internal/index/paths_backfill_test.go new file mode 100644 index 0000000..48ee007 --- /dev/null +++ b/modules/mesh-delivery-indexer/internal/index/paths_backfill_test.go @@ -0,0 +1,67 @@ +package index + +import ( + "context" + "testing" + + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +// Issue 1614: a modules key written before the indexer learned paths (mesh-tools and node-tools live) gets its directory +// from the module's newest build record without waiting for a merge of its repository, once, changing no fingerprint. The +// read of the controller's build records for it goes in R4g with build-agent (the lead's decision, 2026-10-22). + +func heldValue(t *testing.T, st *memStore, name, path string) { + t.Helper() + data, err := delivery.EncodeModule(delivery.Module{Repository: "novox/mesh-tools", Commit: "e0fadc43d9dda0bc24bd19bb05b021dd87da8582", + FP2: "src2:" + hex64('2'), Manifest: hex64('9'), Path: path}) + if err != nil { + t.Fatal(err) + } + st.put(name, data) +} + +func hex64(c byte) string { + b := make([]byte, 64) + for i := range b { + b[i] = c + } + return string(b) +} + +func TestAnEmptyPathIsFilledOnceFromTheModulesNewestBuildRecord(t *testing.T) { // 1614 + ix, mods := modulesIndexer(t, + [2]any{delivery.BuildKey("mesh-tools", "build-1"), buildRecord(t, "novox/mesh-tools", "mesh-tools", "", "2026-10-19T09:00:00Z")}, + [2]any{delivery.BuildKey("node-tools", "build-2"), buildRecord(t, "novox/mesh-tools", "node-tools", "node-tools", "2026-10-19T09:00:00Z")}) + heldValue(t, mods, "mesh-tools", "") + heldValue(t, mods, "node-tools", "") + before, _ := moduleOf(t, mods, "node-tools") + if err := ix.Recompute(context.Background()); err != nil { + t.Fatal(err) + } + for name, want := range map[string]string{"mesh-tools": delivery.RootPath, "node-tools": "node-tools"} { + m, _ := moduleOf(t, mods, name) + if m.Path != want || m.FP2 != before.FP2 || m.Commit != before.Commit { + t.Errorf("%s = %+v, want path %q and nothing else changed", name, m, want) + } + } + writes := mods.writes + if err := ix.Recompute(context.Background()); err != nil { + t.Fatal(err) + } + if mods.writes != writes { + t.Errorf("the paths were written again (%d writes, was %d): once only", mods.writes, writes) + } +} + +func TestASetPathIsNeverOverwrittenByABuildRecord(t *testing.T) { // 1614 + ix, mods := modulesIndexer(t, [2]any{delivery.BuildKey("node-tools", "build-2"), buildRecord(t, "novox/mesh-tools", "node-tools", "elsewhere", "2026-10-19T09:00:00Z")}) + heldValue(t, mods, "node-tools", "node-tools") + writes := mods.writes + if err := ix.Recompute(context.Background()); err != nil { + t.Fatal(err) + } + if m, _ := moduleOf(t, mods, "node-tools"); m.Path != "node-tools" || mods.writes != writes { + t.Errorf("module = %+v after %d writes (was %d), want its path kept and nothing written", m, mods.writes, writes) + } +} diff --git a/modules/mesh-delivery-indexer/internal/index/toolchains.go b/modules/mesh-delivery-indexer/internal/index/toolchains.go index ab29381..5a93dcd 100644 --- a/modules/mesh-delivery-indexer/internal/index/toolchains.go +++ b/modules/mesh-delivery-indexer/internal/index/toolchains.go @@ -134,6 +134,9 @@ func (ix *Indexer) Recompute(ctx context.Context) error { return ix.recomputeWit // of what the bucket holds, are computed with the others, and are written once, with their final fingerprint or reason // (issue 1444: a merge writes each module once). A record of the overlay that has no kept inputs is written as it is. func (ix *Indexer) recomputeWith(ctx context.Context, overlay map[string]delivery.Module) error { + if err := ix.fillPaths(ctx); err != nil { + return err + } if err := ix.recomputeModules(ctx, overlay); err != nil { return err } @@ -420,3 +423,43 @@ func (ix *Indexer) RecomputeForBuild(ctx context.Context) error { } return ix.Recompute(ctx) } + +// fillPaths gives every module whose key names no directory (a key written before the indexer learned paths) the one its +// newest build record names, once, by compare-and-set, changing nothing else (issue 1614): the builder needs it, and a +// rarely merged repository would leave it without one for weeks. A key that names a directory is never written here. +// This read of the controller's build records goes in R4g with build-agent. +func (ix *Indexer) fillPaths(ctx context.Context) error { + if ix.Modules == nil || ix.Builds == nil { + return nil + } + held, err := ix.heldModules(ctx) + if err != nil { + return err + } + for _, key := range slices.Sorted(maps.Keys(held)) { + if held[key].Path != "" { + continue + } + dir, ok := ix.Builds.PathOf(key, held[key].Repository) + if !ok { + continue + } + err := write(ctx, ix.Modules, key, func(value []byte, found bool) ([]byte, bool, error) { + if !found { + return nil, false, nil + } + cur, err := delivery.DecodeModule(value) + if err != nil || cur.Path != "" { + return nil, false, nil + } + cur.Path = dir + data, err := delivery.EncodeModule(cur) + return data, false, err + }) + if err != nil { + return fmt.Errorf("modules.%s: %w", key, err) + } + ix.logf("modules.%s names no directory: %q filled from its newest build record (issue 1614)", key, dir) + } + return nil +}