From eaf27f36af063043e4775bf10669a1c69327e3f1 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 25 Oct 2026 21:00:56 +0100 Subject: [PATCH 1/4] Write a module's value only when the module changed, so a merge no longer rewrites every module of its repository The operator, 2026-10-25: only if a module has a new version in commit x is a new value written naming commit x. A merge wrote every module of the repository at the new commit, fingerprint unchanged (about 186 keys per mesh-catalog merge), and the controller's take-in took each in again: twice tonight it flooded. Now a merge that gives a module the value it holds but for the commit writes nothing, and the value keeps the commit its fingerprint first appeared at. The kind: a write that differs only in where it was computed. The other instance, a source revision re-stamping Src on every module at its repository's head after any merge anywhere, now stamps Src once. The trunk head stays the gate's source.main. --- .../cmd/mesh-delivery-checker/judge.go | 3 +- .../mesh-delivery-indexer/integration_test.go | 45 ++++++++++++---- .../internal/index/modules.go | 3 +- .../internal/index/modules_test.go | 53 ++++++++++++++++--- .../internal/index/source.go | 8 +-- .../internal/index/source_test.go | 6 +-- .../internal/index/toolchains.go | 15 +++++- .../internal/index/toolchains_test.go | 6 ++- .../cmd/mesh-delivery-view-tools/main.go | 2 +- .../mesh-delivery-view/internal/view/read.go | 5 +- 10 files changed, 116 insertions(+), 30 deletions(-) diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge.go index 066488f..904b8b3 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge.go @@ -182,7 +182,8 @@ func (r buildReader) Build(ctx context.Context, module, fp2 string) (delivery.Bu return b, err == nil, err } -// CommitOf is the commit the indexer's record of the module was computed at: the commit of its repository the mesh runs. +// CommitOf is the commit the indexer's record of the module names: the commit its fingerprint first appeared at, so the source +// of the build the mesh runs (a later merge that leaves the module alone writes no record, the operator 2026-10-25). func (r *repoModules) CommitOf(module string) string { r.mu.Lock() defer r.mu.Unlock() 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 d9142a5..20c9fc1 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 @@ -7,8 +7,10 @@ import ( "encoding/json" "fmt" "io" + "maps" "os" "path/filepath" + "slices" "strings" "testing" "time" @@ -438,8 +440,9 @@ func TestIndexerGivesACandidateItsFingerprintWhenTheCheckerCreatesThePreviewsBuc // TestAMergeOfMeshSDKChangingOnlyGoLeavesItsNamedTreeEqual: two merged messages of mesh-sdk (its module.json at 9cb4d59, // whose lib names its paths) differ in the root tree, as a change to go/ alone makes them, and carry the same objects of the -// named paths; the module's kept tree, and so its fingerprint's tree input, is the same after both, and its new form -// (fingerprint or reason) does not move (issue 1758, I9, ADR 0499 check clause 6 in miniature). +// named paths; the module's kept tree, and so its fingerprint's tree input, is the same after both, its new form +// (fingerprint or reason) does not move, and so the second writes no value at all (issue 1758, I9, ADR 0499 check clause 6 +// in miniature; the operator, 2026-10-25: a module's value moves only with the module). func TestAMergeOfMeshSDKChangingOnlyGoLeavesItsNamedTreeEqual(t *testing.T) { manifest, err := os.ReadFile("../../internal/index/testdata/sdk-9cb4d59-mesh-sdk.module.json") if err != nil { @@ -448,7 +451,7 @@ func TestAMergeOfMeshSDKChangingOnlyGoLeavesItsNamedTreeEqual(t *testing.T) { objects := map[string]string{"src": "1111111111111111111111111111111111111111", "test": "2222222222222222222222222222222222222222", "package.json": "3333333333333333333333333333333333333333", "package-lock.json": "4444444444444444444444444444444444444444", "tsconfig.json": "5555555555555555555555555555555555555555"} js, ctx, _, modules := startIndexer(t, func(context.Context, jetstream.KeyValue) {}) - merged := func(merge, root string) delivery.Module { + publish := func(merge, root string, objects map[string]string) { data, err := delivery.EncodeForge(delivery.Forge{Kind: delivery.ForgeMerged, Repository: "novox/mesh-sdk", Branch: "feat/x", Number: 70, Head: "0123456789abcdef0123456789abcdef01234567", Merge: merge, Root: root, Trees: map[string]string{"": root}, Manifests: map[string]string{"": string(manifest)}, Paths: map[string]map[string]string{"": objects}}) @@ -458,6 +461,8 @@ func TestAMergeOfMeshSDKChangingOnlyGoLeavesItsNamedTreeEqual(t *testing.T) { if _, err := js.Publish(ctx, "mesh.stream.mesh-delivery-forge-bridge.event.forge.novox.mesh-sdk.merged", data); err != nil { t.Fatal(err) } + } + at := func(merge string) delivery.Module { var m delivery.Module for deadline := time.Now().Add(5 * time.Second); time.Now().Before(deadline); time.Sleep(50 * time.Millisecond) { if e, err := modules.Get(ctx, "mesh-sdk"); err == nil { @@ -469,13 +474,35 @@ func TestAMergeOfMeshSDKChangingOnlyGoLeavesItsNamedTreeEqual(t *testing.T) { t.Fatalf("modules.mesh-sdk = %+v within 5 s, want the merge %s", m, merge) return m } - first := merged("89abcdef0123456789abcdef0123456789abcdef", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa") - second := merged("99abcdef0123456789abcdef0123456789abcdef", "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb") - if first.Inputs2 == nil || second.Inputs2 == nil || first.Inputs2.Tree != second.Inputs2.Tree || !strings.HasPrefix(first.Inputs2.Tree, delivery.NamedTreePrefix) { - t.Fatalf("kept trees %+v and %+v, want one named tree", first.Inputs2, second.Inputs2) + const firstMerge, secondMerge, thirdMerge = "89abcdef0123456789abcdef0123456789abcdef", "99abcdef0123456789abcdef0123456789abcdef", "a9abcdef0123456789abcdef0123456789abcdef" + publish(firstMerge, "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", objects) + first := at(firstMerge) + if first.Inputs2 == nil || !strings.HasPrefix(first.Inputs2.Tree, delivery.NamedTreePrefix) { + t.Fatalf("kept inputs %+v, want a named tree", first.Inputs2) } - if first.FP2 != second.FP2 || first.None2 != second.None2 { - t.Errorf("the new form moved: %q/%q, then %q/%q", first.FP2, first.None2, second.FP2, second.None2) + // The go/-only merge, then one that changes src/: the stream is taken in order, so once the third is written the second + // was heard, and it wrote nothing: the module's value stays at the commit its fingerprint first appeared at (the + // operator, 2026-10-25). + publish(secondMerge, "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", objects) + changed := maps.Clone(objects) + changed["src"] = "6666666666666666666666666666666666666666" + publish(thirdMerge, "cccccccccccccccccccccccccccccccccccccccc", changed) + at(thirdMerge) + history, err := modules.History(ctx, "mesh-sdk") + if err != nil { + t.Fatal(err) + } + var commits []string + for _, e := range history { + if m, err := delivery.DecodeModule(e.Value()); err == nil { + commits = append(commits, m.Commit) + if m.Commit == firstMerge && (m.Inputs2 == nil || m.Inputs2.Tree != first.Inputs2.Tree || m.FP2 != first.FP2 || m.None2 != first.None2) { + t.Errorf("the value at the first merge moved: %+v, was %+v", m, first) + } + } + } + if slices.Contains(commits, secondMerge) { + t.Errorf("the go/-only merge wrote a value: the commits written are %v", commits) } } diff --git a/modules/mesh-delivery-indexer/internal/index/modules.go b/modules/mesh-delivery-indexer/internal/index/modules.go index b37c893..5ff6464 100644 --- a/modules/mesh-delivery-indexer/internal/index/modules.go +++ b/modules/mesh-delivery-indexer/internal/index/modules.go @@ -22,7 +22,8 @@ import ( // form (Module.FP, None) is no longer written: it was computed from those records (ADR 0453 clause 6). A module the // bucket holds of another repository is left as it is and said: a move between repositories is the operator's (ADR 0420 // clause 7). Two directories naming one module write the one the bucket holds, or the first by name, and say the other. -// A value the module already holds is not written again. +// A value the module already holds but for its commit is not written again: its commit stays the one its fingerprint first +// appeared at (writeOverlay), so a merge writes only the modules it changes. func (ix *Indexer) applyMerged(ctx context.Context, f delivery.Forge) error { digests, err := ix.keepManifests(ctx, f) if err != nil { diff --git a/modules/mesh-delivery-indexer/internal/index/modules_test.go b/modules/mesh-delivery-indexer/internal/index/modules_test.go index ca0cf36..18987ef 100644 --- a/modules/mesh-delivery-indexer/internal/index/modules_test.go +++ b/modules/mesh-delivery-indexer/internal/index/modules_test.go @@ -85,14 +85,51 @@ func moduleOf(t *testing.T, st *memStore, key string) (delivery.Module, bool) { const aInputs = `{"Module":"a","Tree":"oldtree","Bases":["registry.example/base@sha256:aaa"],"Contexts":null,"Toolchains":{"code":"go sha256:ccc 0123456789abcdef"},"Unpinned":""}` -func TestAnUnchangedModuleWritesNothing(t *testing.T) { // 1239 (unchanged value writes nothing) - ix, mods := modulesIndexer(t, rec(t, "build-1", "a", "novox/mesh-delivery", "modules/a", "main", "", "2026-10-19T09:00:00Z", aInputs)) - m := mergedMsg("novox/mesh-delivery", map[string]string{"modules/a": "newtree"}) - _ = ix.Apply(context.Background(), m) - rev, writes := mods.revs["a"], mods.writes - _ = ix.Apply(context.Background(), m) - if mods.revs["a"] != rev || mods.writes != writes { - t.Errorf("the same merge wrote again (%d writes, was %d)", mods.writes, writes) +func TestAnUnchangedModuleWritesNothing(t *testing.T) { // 1239 (unchanged value writes nothing); the operator 2026-10-25 (a module's value moves only with the module) + const repo, next = "novox/mesh-delivery", "cccccccccccccccccccccccccccccccccccccccc" + three := map[string]string{"modules/a": "treeA", "modules/b": "treeB", "modules/c": "treeC"} + for _, tc := range []struct { + name string + then func(ix *Indexer) error // after the first merge of a, b and c at merge, stamped by source revision 7 + // writes is the values written by then; commit is each module's commit after. + writes int + commit map[string]string + }{ + {name: "the same merge again", writes: 0, commit: map[string]string{"a": merge, "b": merge, "c": merge}, + then: func(ix *Indexer) error { return ix.Apply(context.Background(), mergedMsg(repo, three)) }}, + {name: "a merge touching one of three modules", writes: 1, commit: map[string]string{"a": next, "b": merge, "c": merge}, + then: func(ix *Indexer) error { + m := mergedMsg(repo, map[string]string{"modules/a": "treeA2", "modules/b": "treeB", "modules/c": "treeC"}) + m.Merge = next + return ix.Apply(context.Background(), m) + }}, + {name: "a later source revision of another repository's merge", writes: 0, commit: map[string]string{"a": merge, "b": merge, "c": merge}, + then: func(ix *Indexer) error { + return ix.ApplySource(context.Background(), 9, delivery.Source{Group: "g", Heads: map[string]string{repo: merge, "novox/mesh-catalog": otherMerge}}) + }}, + } { + t.Run(tc.name, func(t *testing.T) { + ix, mods := modulesIndexer(t) + ctx := context.Background() + if err := ix.Apply(ctx, mergedMsg(repo, three)); err != nil { + t.Fatal(err) + } + if err := ix.ApplySource(ctx, 7, sourceOf(repo, merge)); err != nil { + t.Fatal(err) + } + writes := mods.writes + if err := tc.then(ix); err != nil { + t.Fatal(err) + } + if got := mods.writes - writes; got != tc.writes { + t.Errorf("%d values written, want %d", got, tc.writes) + } + for module, want := range tc.commit { + if m, ok := moduleOf(t, mods, module); !ok || m.Commit != want { + t.Errorf("%s = %+v (%v), want commit %.8s", module, m, ok, want) + } + } + }) } } diff --git a/modules/mesh-delivery-indexer/internal/index/source.go b/modules/mesh-delivery-indexer/internal/index/source.go index 0f2c781..38869ec 100644 --- a/modules/mesh-delivery-indexer/internal/index/source.go +++ b/modules/mesh-delivery-indexer/internal/index/source.go @@ -32,8 +32,8 @@ func (ix *Indexer) srcOf(repository, merge string) uint64 { } // ApplySource takes a revision of the gate's source.main in. An older revision than the latest seen is left. For each -// repository the source names, each module built from it whose fingerprint was computed at the head the source names is -// written again with Src the revision; a module computed at another commit is left (a merge heard later is stamped +// repository the source names, each module built from it whose fingerprint was computed at the head the source names, and +// that carries no Src yet, is written again with Src the revision (once: a later revision leaves it); a module computed at another commit is left (a merge heard later is stamped // when it is written, applyMerged), and a repository none of the indexer's modules is built from writes nothing. The // modules of a repository are those the modules bucket holds of it (ADR 0501 clause 6). func (ix *Indexer) ApplySource(ctx context.Context, rev uint64, s delivery.Source) error { @@ -71,7 +71,9 @@ func (ix *Indexer) ApplySource(ctx context.Context, rev uint64, s delivery.Sourc ix.logf("modules.%s holds what is not a module, left: %v", key, err) return nil, false, nil } - if cur.Repository != repository || cur.Commit != head || cur.Src == rev { + // Stamped once, with the first revision naming the commit as the head: a later revision (a merge of another + // repository) leaves the value as it is, so no revision of the source rewrites a module that did not change. + if cur.Repository != repository || cur.Commit != head || cur.Src != 0 { return nil, false, nil } cur.Src = rev diff --git a/modules/mesh-delivery-indexer/internal/index/source_test.go b/modules/mesh-delivery-indexer/internal/index/source_test.go index d7b16a5..45260ab 100644 --- a/modules/mesh-delivery-indexer/internal/index/source_test.go +++ b/modules/mesh-delivery-indexer/internal/index/source_test.go @@ -38,15 +38,15 @@ func TestASourceRevisionStampsTheModulesOfTheRepositoriesHeadItNames(t *testing. if c, ok := moduleOf(t, mods, "c"); ok { t.Errorf("module c = %+v: written for a head no merge message of the indexer carried", c) } - // The same revision again changes nothing; a later revision moves Src. + // The same revision again changes nothing, and so does a later one: Src is the first revision naming the commit. if err := ix.ApplySource(ctx, 7, s); err != nil { t.Fatal(err) } if err := ix.ApplySource(ctx, 9, s); err != nil { t.Fatal(err) } - if m, _ := moduleOf(t, mods, "a"); m.Src != 9 { - t.Errorf("module a Src = %d, want 9", m.Src) + if m, _ := moduleOf(t, mods, "a"); m.Src != 7 { + t.Errorf("module a Src = %d, want 7", m.Src) } } diff --git a/modules/mesh-delivery-indexer/internal/index/toolchains.go b/modules/mesh-delivery-indexer/internal/index/toolchains.go index cbf1794..c1a64fa 100644 --- a/modules/mesh-delivery-indexer/internal/index/toolchains.go +++ b/modules/mesh-delivery-indexer/internal/index/toolchains.go @@ -319,6 +319,12 @@ func (ix *Indexer) recomputeModules(ctx context.Context, overlay map[string]deli // writeOverlay writes the records of a merge, each once and only if it differs from what the bucket holds; finals has // the ones that were computed, the others are written as the merge gave them. +// +// **A module the merge did not change is not written** (the operator, 2026-10-25: "only if a module has got a new version in +// commit x, should a new module version get created linked to commit x"). A value names the commit its fingerprint first +// appeared at: when the merge gives a module the value it holds but for the commit (and the source revision stamped on it), +// the held value stands, its commit unchanged, so a catalogue merge touching one module writes one key, not one per module of +// the repository, and the controller's take-in hears only that one. The repository's trunk head is the gate's source.main. func (ix *Indexer) writeOverlay(ctx context.Context, overlay, finals map[string]delivery.Module) error { for _, key := range slices.Sorted(maps.Keys(overlay)) { next := overlay[key] @@ -327,7 +333,7 @@ func (ix *Indexer) writeOverlay(ctx context.Context, overlay, finals map[string] } err := write(ctx, ix.Modules, key, func(value []byte, found bool) ([]byte, bool, error) { if found { - if cur, err := delivery.DecodeModule(value); err == nil && reflect.DeepEqual(cur, withV(next)) { + if cur, err := delivery.DecodeModule(value); err == nil && sameButWhere(cur, next) { return nil, false, nil } } @@ -341,6 +347,13 @@ func (ix *Indexer) writeOverlay(ctx context.Context, overlay, finals map[string] return nil } +// sameButWhere says the held value cur is next but for where it was computed (Commit and Src): the module is unchanged. +func sameButWhere(cur, next delivery.Module) bool { + next = withV(next) + next.Commit, next.Src = cur.Commit, cur.Src + return reflect.DeepEqual(cur, next) +} + // manifestRaw is manifests.: the manifest as stored and parsed by the running SDK. func (ix *Indexer) manifestRaw(ctx context.Context, digest string) (string, catalogue.Manifest, error) { key, err := delivery.ManifestsKey(digest) diff --git a/modules/mesh-delivery-indexer/internal/index/toolchains_test.go b/modules/mesh-delivery-indexer/internal/index/toolchains_test.go index df6128a..b9d01be 100644 --- a/modules/mesh-delivery-indexer/internal/index/toolchains_test.go +++ b/modules/mesh-delivery-indexer/internal/index/toolchains_test.go @@ -267,11 +267,13 @@ func TestAMergeKeepsAFingerprintTheModuleStillHas(t *testing.T) { // 1444 (revie w := mods.writes // the same catalogue merged again at a later commit: nothing moved _, _, f := newFormIndexerUnmerged(t, builderModules(t)...) + f.Merge = otherMerge if err := ix.Apply(ctx, f); err != nil { t.Fatal(err) } - if after, _ := moduleOf(t, mods, "mesh-tools"); after.FP2 != before.FP2 || mods.writes != w { - t.Errorf("fp2 = %q (was %q), writes %d more", after.FP2, before.FP2, mods.writes-w) + // and the commit stays the one the fingerprint first appeared at (the operator, 2026-10-25) + if after, _ := moduleOf(t, mods, "mesh-tools"); after.FP2 != before.FP2 || after.Commit != before.Commit || mods.writes != w { + t.Errorf("fp2 = %q at %.8s (was %q at %.8s), writes %d more", after.FP2, after.Commit, before.FP2, before.Commit, mods.writes-w) } } diff --git a/modules/mesh-delivery-view/cmd/mesh-delivery-view-tools/main.go b/modules/mesh-delivery-view/cmd/mesh-delivery-view-tools/main.go index c277b4b..3ffe7d1 100644 --- a/modules/mesh-delivery-view/cmd/mesh-delivery-view-tools/main.go +++ b/modules/mesh-delivery-view/cmd/mesh-delivery-view-tools/main.go @@ -72,7 +72,7 @@ func Tools(r view.Reader) []stdio.Tool { }}, {Name: view.BuilderBuildReadTool, Description: "One build of the builder as stored. Give the module and its fingerprint (fp2, src2:...), or the module alone for the build " + - "at the fingerprint the indexer holds for it now, joined through modules.; with commit, only if the indexer holds the module at that commit. Read-only.", + "at the fingerprint the indexer holds for it now, joined through modules.; with commit, only if the indexer holds the module at that commit (a module's value names the commit its fingerprint first appeared at, and stays there through merges that leave the module alone). Read-only.", Input: map[string]any{"type": "object", "required": []string{"module"}, "properties": map[string]any{ "module": map[string]any{"type": "string", "description": "the module's name"}, "fp2": map[string]any{"type": "string", "description": "its new-form fingerprint; empty means the indexer's current one"}, diff --git a/modules/mesh-delivery-view/internal/view/read.go b/modules/mesh-delivery-view/internal/view/read.go index 55ab022..8dd8342 100644 --- a/modules/mesh-delivery-view/internal/view/read.go +++ b/modules/mesh-delivery-view/internal/view/read.go @@ -311,7 +311,10 @@ func (r Reader) BuilderBuild(module, fp2, commit string) (BuilderRow, error) { return BuilderRow{}, fmt.Errorf("the indexer's value of %s is not readable: %w", module, err) } if commit != "" && m.Commit != commit { - return BuilderRow{}, fmt.Errorf("the indexer holds %s at commit %s, not %s, so its fingerprint is not the one of that commit", module, m.Commit, commit) + // The value names the commit its fingerprint first appeared at, and a later merge that left the module alone + // writes none (the operator, 2026-10-25): another commit may well have the same fingerprint, which is not guessed. + return BuilderRow{}, fmt.Errorf("the indexer holds %s at commit %s, the commit its fingerprint first appeared at, not %s; "+ + "a later trunk commit that left the module alone keeps that fingerprint: ask without commit for the build at it", module, m.Commit, commit) } if m.FP2 == "" { return BuilderRow{}, fmt.Errorf("the indexer holds no new-form fingerprint of %s: %s", module, m.None2) -- 2.54.0 From 9afef800c09f1d437d056d18e31c91a5f1672638 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 25 Oct 2026 21:10:49 +0100 Subject: [PATCH 2/4] Merge a candidate's preview onto the trunk head it was previewed onto, not the module value's commit The builder took a candidate build's merge base from the module's value commit. Now that a value keeps the commit its fingerprint first appeared at, that commit can be many merges behind the trunk, so a preview would be built onto a stale trunk (review round 1). The base is now the head of the pull request's repository in the gate's source revision the candidate names (Candidate.Base): the builder reads mesh-delivery-gate.source, watched with its history (a direct get of one revision is not what a read grants), and a candidate heard before its revision is built once the revision is heard. --- .../cmd/mesh-delivery-builder/bus.go | 5 ++ .../mesh-delivery-builder/integration_test.go | 31 +++++-- .../cmd/mesh-delivery-builder/main.go | 82 ++++++++++++++++++- .../internal/build/candidates.go | 25 +++++- .../internal/build/candidates_test.go | 11 ++- .../internal/build/r4e_test.go | 5 +- .../internal/build/run.go | 10 +++ .../internal/build/run_test.go | 1 + .../mesh-delivery-builder/manifest_test.go | 2 +- modules/mesh-delivery-builder/module.json | 1 + 10 files changed, 159 insertions(+), 14 deletions(-) diff --git a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/bus.go b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/bus.go index 359b1e9..2f32c84 100644 --- a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/bus.go +++ b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/bus.go @@ -66,6 +66,11 @@ func settle(ctx context.Context, kv jetstream.KeyValue, each func(jetstream.KeyV if err != nil { return nil, err } + return settled(ctx, w, each) +} + +// settled hands each entry the watch delivers before its end-of-initial marker to each, and answers the watch. +func settled(ctx context.Context, w jetstream.KeyWatcher, each func(jetstream.KeyValueEntry)) (jetstream.KeyWatcher, error) { for { select { case <-ctx.Done(): diff --git a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/integration_test.go b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/integration_test.go index 5a37fc5..5184160 100644 --- a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/integration_test.go +++ b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/integration_test.go @@ -162,11 +162,29 @@ func (f *fakeBuildModules) serve(t *testing.T, module string, nodes ...string) { } } +// trunkHead is the trunk head of novox/mesh-catalog the bed's source revision names: newer than the module values' commit. +const trunkHead = "3333333333333333333333333333333333333333" + +// trunk writes the gate's source.main naming head as novox/mesh-catalog's trunk head, and answers its revision. +func (b *bed) trunk(t *testing.T, head string) uint64 { + t.Helper() + data, err := delivery.EncodeSource(delivery.Source{V: delivery.Version, Group: "g", Heads: map[string]string{"novox/mesh-catalog": head}}) + if err != nil { + t.Fatal(err) + } + rev, err := b.source.Put(b.ctx, delivery.SourceMainKey, data) + if err != nil { + t.Fatal(err) + } + return rev +} + type bed struct { ctx context.Context js jetstream.JetStream modules jetstream.KeyValue candidates jetstream.KeyValue + source jetstream.KeyValue builds jetstream.KeyValue tools map[string]jetstream.KeyValue instances jetstream.KeyValue @@ -243,6 +261,7 @@ func startBedWith(t *testing.T, closed bool) *bed { } b.modules = mk(delivery.ModulesBucket, delivery.ModulesHistory) b.candidates = mk(delivery.CandidatesBucket, delivery.CandidatesHistory) + b.source = mk(delivery.SourceBucket, delivery.SourceHistory) mk(delivery.ManifestsBucket, delivery.ManifestsHistory) b.builds = mk(delivery.BuilderBuildsBucket, delivery.BuilderBuildsHistory) b.instances = mk(instancesBucket, 1) @@ -513,7 +532,9 @@ func TestAKeyLeftBuildingForAModuleThatIsGoneIsClosedAtStart(t *testing.T) { // TestBuilderBuildsACandidatesFingerprintFromTheHeadMergedOntoTheTrunk: the trunk's record of netcheck carries a fingerprint of its // own; a candidate the indexer wrote gives netcheck the fingerprint fp at a pull request's head; the program writes -// builds.netcheck. done, its fetch given the head, the ref that carries it and the trunk commit (issue 1684). +// builds.netcheck. done, its fetch given the head, the ref that carries it and the trunk head of the candidate's source +// revision, newer than the module value's commit (issue 1684; the operator 2026-10-25: a value names the commit its +// fingerprint first appeared at). func TestBuilderBuildsACandidatesFingerprintFromTheHeadMergedOntoTheTrunk(t *testing.T) { // N7/AC1 b := startBed(t) other := "src2:" + strings.Repeat("e", 64) @@ -530,7 +551,7 @@ func TestBuilderBuildsACandidatesFingerprintFromTheHeadMergedOntoTheTrunk(t *tes pulls := []delivery.CandidatePull{{Head: delivery.Head{Repository: "novox/mesh-catalog", Number: 7, Head: head}, Modules: []string{"netcheck"}, Fps: map[string]delivery.CandidateModule{"netcheck": {FP2: fp, Manifest: digest}}}} hash, _ := delivery.HeadsHash([]delivery.Head{pulls[0].Head}) - cand, err := delivery.EncodeCandidate(delivery.Candidate{V: delivery.Version, Group: "feat/x", HeadsHash: hash, Pulls: pulls, FPSet: delivery.FingerprintSet(pulls)}) + cand, err := delivery.EncodeCandidate(delivery.Candidate{V: delivery.Version, Group: "feat/x", Base: b.trunk(t, trunkHead), HeadsHash: hash, Pulls: pulls, FPSet: delivery.FingerprintSet(pulls)}) if err != nil { t.Fatal(err) } @@ -550,12 +571,12 @@ func TestBuilderBuildsACandidatesFingerprintFromTheHeadMergedOntoTheTrunk(t *tes defer b.fake.mu.Unlock() found := false for _, f := range b.fake.fetches { - if f["commit"] == head && f["ref"] == "refs/pull/7/head" && f["base"] == commit { + if f["commit"] == head && f["ref"] == "refs/pull/7/head" && f["base"] == trunkHead { found = true } } if !found { - t.Errorf("fetches = %v, want one of the candidate's head with its ref and the trunk commit", b.fake.fetches) + t.Errorf("fetches = %v, want one of the candidate's head with its ref and the trunk head it was previewed onto, not the module value's commit", b.fake.fetches) } } @@ -603,7 +624,7 @@ func TestAnImageIsBuiltOnOneNodeAndOnlyATrunkKeyPublishes(t *testing.T) { // 170 pulls := []delivery.CandidatePull{{Head: delivery.Head{Repository: "novox/mesh-catalog", Number: 7, Head: head}, Modules: []string{"imaged"}, Fps: map[string]delivery.CandidateModule{"imaged": {FP2: fp, Manifest: digest}}}} hash, _ := delivery.HeadsHash([]delivery.Head{pulls[0].Head}) - cand, err := delivery.EncodeCandidate(delivery.Candidate{V: delivery.Version, Group: "feat/x", HeadsHash: hash, Pulls: pulls, FPSet: delivery.FingerprintSet(pulls)}) + cand, err := delivery.EncodeCandidate(delivery.Candidate{V: delivery.Version, Group: "feat/x", Base: b.trunk(t, trunkHead), HeadsHash: hash, Pulls: pulls, FPSet: delivery.FingerprintSet(pulls)}) if err != nil { t.Fatal(err) } diff --git a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/main.go b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/main.go index b146faa..77cbc22 100644 --- a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/main.go +++ b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/main.go @@ -16,9 +16,12 @@ import ( "git.novox.be/novox/mesh-sdk/go/delivery/buildrun" "io" "log" + "maps" "os" "os/signal" + "slices" "strings" + "sync" "syscall" "time" @@ -174,6 +177,27 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro Logf: logger.Printf, Stored: storedVia(calls.Call), } + // A candidate's preview is merged onto the trunk head the indexer previewed it onto: source.main at the candidate's base + // revision (the gate's source state). Where the bucket is not there yet no candidate's fingerprint is built, and said. + // The source's revisions are watched with their history (a direct get of one revision is not what a read grants): a + // revision heard after the candidate that names it builds that candidate then. + trunks := &sourceHeads{} + var sw jetstream.KeyWatcher + switch kv, err := js.KeyValue(ctx, delivery.SourceBucket); { + case errors.Is(err, jetstream.ErrBucketNotFound): + logger.Printf("the bucket %s does not exist yet: no candidate's fingerprint is built until the builder starts again", delivery.SourceBucket) + case err != nil: + return fmt.Errorf("the bucket %s: %w", delivery.SourceBucket, err) + default: + w, err := kv.Watch(ctx, delivery.SourceMainKey, jetstream.IncludeHistory()) + if err != nil { + return fmt.Errorf("the bucket %s cannot be watched: %w", delivery.SourceBucket, err) + } + if sw, err = settled(ctx, w, func(e jetstream.KeyValueEntry) { trunks.put(e) }); err != nil { + return err + } + b.Trunk = trunks.head + } // **Said once every build module answered a reference, taken away when stopped** (issue 1886, ADR 0522): the // controller delivers from this builder's keys only while an instance says it writes references. A bucket not there // yet (the manifest's state not made) is said, and the controller's plan waits for the keys, named (no build-agent @@ -267,9 +291,18 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro return err } logger.Printf("started on %s", cred.Node) - errs := make(chan error, 2) + errs := make(chan error, 3) go func() { errs <- follow(ctx, w, each) }() go func() { errs <- follow(ctx, cw, eachCandidate) }() + if sw != nil { + go func() { + errs <- follow(ctx, sw, func(e jetstream.KeyValueEntry) { + if trunks.put(e) { + b.TrunkMoved(ctx) + } + }) + }() + } select { case <-ctx.Done(): b.Wait() @@ -282,3 +315,50 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro return err } } + +// sourceHeads are the trunk heads of each revision of the gate's source.main the watch delivered, the newest +// delivery.SourceHistory of them: what a candidate's base revision names. +type sourceHeads struct { + mu sync.Mutex + byRev map[uint64]map[string]string +} + +// put takes one revision in and answers whether it is new. +func (s *sourceHeads) put(e jetstream.KeyValueEntry) bool { + if !opOf(e) { + return false + } + src, err := delivery.DecodeSource(e.Value()) + if err != nil { + return false + } + s.mu.Lock() + defer s.mu.Unlock() + if s.byRev == nil { + s.byRev = map[uint64]map[string]string{} + } + if _, had := s.byRev[e.Revision()]; had { + return false + } + s.byRev[e.Revision()] = src.Heads + for len(s.byRev) > delivery.SourceHistory { + delete(s.byRev, slices.Min(slices.Collect(maps.Keys(s.byRev)))) + } + return true +} + +// head is the head of repository in revision rev; an error when the revision is not heard (yet, or any more). +func (s *sourceHeads) head(_ context.Context, rev uint64, repository string) (string, error) { + s.mu.Lock() + defer s.mu.Unlock() + heads, ok := s.byRev[rev] + if !ok { + return "", fmt.Errorf("source revision %d is not heard (it is built once it is)", rev) + } + for r, head := range heads { + if delivery.SameRepository(r, repository) { + return head, nil + } + } + return "", nil +} diff --git a/modules/mesh-delivery-builder/internal/build/candidates.go b/modules/mesh-delivery-builder/internal/build/candidates.go index d96df2c..108c149 100644 --- a/modules/mesh-delivery-builder/internal/build/candidates.go +++ b/modules/mesh-delivery-builder/internal/build/candidates.go @@ -73,9 +73,17 @@ func (b *Builder) considerCandidate(ctx context.Context, c delivery.Candidate) { // The named paths' objects the preview gave it: the fetch names the paths (runLane, ADR 0499 clause 4). m.Inputs2 = &delivery.KeptInputs2{Paths: cm.Paths} } + // The preview is merged onto the trunk head the candidate was previewed onto: the head of the pull request's + // repository in the gate's source revision c.Base (ADR 0484 §3). Never the module's value's commit: that is the + // commit its fingerprint first appeared at, and may be many merges behind the trunk. + base, err := b.trunkHead(ctx, c.Base, p.Repository) + if err != nil { + b.logf("candidates.%s: the module %s's fingerprint %s is not built: %v", c.Group, name, cm.FP2, err) + continue + } b.mu.Lock() if _, onTrunk := b.trunkFP[key]; !onTrunk { - b.via[key] = via{Ref: fmt.Sprintf("refs/pull/%d/head", p.Number), Base: trunk.Commit} + b.via[key] = via{Ref: fmt.Sprintf("refs/pull/%d/head", p.Number), Base: base} } b.mu.Unlock() b.consider(ctx, name, m) @@ -106,3 +114,18 @@ func (b *Builder) agreedFor(ctx context.Context, manifest string) map[string]del } return out } + +// trunkHead is the head of repository in the gate's source revision rev (Builder.Trunk), or why it cannot be said. +func (b *Builder) trunkHead(ctx context.Context, rev uint64, repository string) (string, error) { + if b.Trunk == nil { + return "", fmt.Errorf("the gate's source cannot be read here, so the trunk head %s was previewed onto is not known", repository) + } + head, err := b.Trunk(ctx, rev, repository) + if err != nil { + return "", fmt.Errorf("the trunk head of %s at source revision %d cannot be read: %w", repository, rev, err) + } + if head == "" { + return "", fmt.Errorf("source revision %d names no trunk head of %s", rev, repository) + } + return head, nil +} diff --git a/modules/mesh-delivery-builder/internal/build/candidates_test.go b/modules/mesh-delivery-builder/internal/build/candidates_test.go index 97ff67a..af8d616 100644 --- a/modules/mesh-delivery-builder/internal/build/candidates_test.go +++ b/modules/mesh-delivery-builder/internal/build/candidates_test.go @@ -40,8 +40,11 @@ func candidateValue(t *testing.T, group string, number int, head string) []byte } const ( - headX = "1111111111111111111111111111111111111111" - headY = "2222222222222222222222222222222222222222" + // trunkHead is the trunk head the rig's source revision names: newer than the module value's commit, which stays at the + // commit its fingerprint first appeared at. + trunkHead = "3333333333333333333333333333333333333333" + headX = "1111111111111111111111111111111111111111" + headY = "2222222222222222222222222222222222222222" ) func TestACandidatesFingerprintIsBuiltOnceAsAKeyOfItsOwnFromTheHeadMergedOntoTheTrunk(t *testing.T) { // N7/AC1, N7/AC2 (issue 1684) @@ -58,8 +61,8 @@ func TestACandidatesFingerprintIsBuiltOnceAsAKeyOfItsOwnFromTheHeadMergedOntoThe t.Fatalf("%d fetches, want one", len(fetches)) } f := fetches[0].body.(map[string]any) - if f["commit"] != headX || f["ref"] != "refs/pull/7/head" || f["base"] != "7a74cee83f88e6585a5511c3cc53bdf0f5303d20" || f["repository"] != "novox/mesh-catalog" { - t.Errorf("fetch args = %+v, want the head, its ref and the trunk commit to merge onto", f) + if f["commit"] != headX || f["ref"] != "refs/pull/7/head" || f["base"] != trunkHead || f["repository"] != "novox/mesh-catalog" { + t.Errorf("fetch args = %+v, want the head, its ref and the trunk head the candidate was previewed onto", f) } if b := r.bm.callsOf("build_go_build")[0].body.(map[string]any); b["module"] != "modules/m" { t.Errorf("build args = %+v", b) diff --git a/modules/mesh-delivery-builder/internal/build/r4e_test.go b/modules/mesh-delivery-builder/internal/build/r4e_test.go index fab0854..a28bfda 100644 --- a/modules/mesh-delivery-builder/internal/build/r4e_test.go +++ b/modules/mesh-delivery-builder/internal/build/r4e_test.go @@ -379,8 +379,9 @@ func TestACandidatesFetchNamesTheNamedPathsTheIndexerGaveIt(t *testing.T) { // A t.Fatal("no fetch") } req, err := buildstep.ParseFetchRequest(f[0].body.(map[string]any)) - if err != nil || req.Dir != "modules/m" || strings.Join(req.Paths, ",") != "package.json,src" || req.Base == "" { - t.Errorf("fetch request = %+v, %v; want the candidate's base, the module's directory and its named paths", req, err) + // The base is the trunk head the candidate was previewed onto, never the module value's commit (an older one). + if err != nil || req.Dir != "modules/m" || strings.Join(req.Paths, ",") != "package.json,src" || req.Base != trunkHead { + t.Errorf("fetch request = %+v, %v; want the trunk head %.8s as the base, the module's directory and its named paths", req, err, trunkHead) } } diff --git a/modules/mesh-delivery-builder/internal/build/run.go b/modules/mesh-delivery-builder/internal/build/run.go index 1d140d7..492cae5 100644 --- a/modules/mesh-delivery-builder/internal/build/run.go +++ b/modules/mesh-delivery-builder/internal/build/run.go @@ -53,6 +53,9 @@ type Builder struct { // Stored answers which references the artifact store no longer holds (the controller's verb held; issue 1937): a done // key of the trunk's fingerprint is asked before it is left done (ClaimOptions.Stored). Nil asks nothing. Stored func(ctx context.Context, references []string) (gone []string, err error) + // Trunk answers the head of a repository in the gate's source revision rev (source.main at that revision): the trunk a + // candidate's preview is merged onto (Candidate.Base). Nil builds no candidate's fingerprint, and says why. + Trunk func(ctx context.Context, rev uint64, repository string) (string, error) once sync.Once sem chan struct{} @@ -269,6 +272,13 @@ func (b *Builder) Reconsider(ctx context.Context) { for _, k := range sortedKeys(known) { b.consider(ctx, k, known[k]) } + b.TrunkMoved(ctx) +} + +// TrunkMoved considers every candidate again once a revision of the gate's source is heard: a candidate heard before the +// revision it was previewed onto had no trunk head to be merged onto. +func (b *Builder) TrunkMoved(ctx context.Context) { + b.init() b.mu.Lock() cands := make([]delivery.Candidate, 0, len(b.cands)) for _, c := range b.cands { diff --git a/modules/mesh-delivery-builder/internal/build/run_test.go b/modules/mesh-delivery-builder/internal/build/run_test.go index f3e82b8..f4ad7b2 100644 --- a/modules/mesh-delivery-builder/internal/build/run_test.go +++ b/modules/mesh-delivery-builder/internal/build/run_test.go @@ -176,6 +176,7 @@ func newRig(t *testing.T, artifacts ...catalogue.Artifact) *rig { r.b = &Builder{ Claimer: Claimer{Store: r.st, By: "novox.builder"}, Asks: buildrun.Asker{Publisher: r.asks, Waiter: r.asks, Bound: 2 * time.Second}, Toolchains: r.tools, Manifests: r.mf, Logf: t.Logf, + Trunk: func(context.Context, uint64, string) (string, error) { return trunkHead, nil }, } var err error if r.key, err = delivery.BuilderBuildKey("m", fp1); err != nil { diff --git a/modules/mesh-delivery-builder/manifest_test.go b/modules/mesh-delivery-builder/manifest_test.go index e5dd046..27e44e9 100644 --- a/modules/mesh-delivery-builder/manifest_test.go +++ b/modules/mesh-delivery-builder/manifest_test.go @@ -90,7 +90,7 @@ func TestManifestDeclarations(t *testing.T) { // 1607/AC1 // 1887/A4c: the builder chooses no node, so it reads neither presence nor have, invokes no build module's tool and // holds no capacity; it puts asks on the three build seats and reads their steps (ADR 0525 §1, §4, §5). if want := []string{"mesh-build-go.steps", "mesh-build-go.toolchains", "mesh-build-image.steps", "mesh-build-node.steps", "mesh-build-node.toolchains", "mesh-controller.assignments", - "mesh-delivery-indexer.candidates", "mesh-delivery-indexer.manifests", "mesh-delivery-indexer.modules"}; !reflect.DeepEqual(sorted(m.Reads), want) { + "mesh-delivery-gate.source", "mesh-delivery-indexer.candidates", "mesh-delivery-indexer.manifests", "mesh-delivery-indexer.modules"}; !reflect.DeepEqual(sorted(m.Reads), want) { t.Errorf("reads = %v, want %v", m.Reads, want) } if want := []string{"node-build-go", "node-build-image", "node-build-node"}; !reflect.DeepEqual(sorted(m.Uses), want) { diff --git a/modules/mesh-delivery-builder/module.json b/modules/mesh-delivery-builder/module.json index 0333c9a..e15c633 100644 --- a/modules/mesh-delivery-builder/module.json +++ b/modules/mesh-delivery-builder/module.json @@ -21,6 +21,7 @@ "mesh-build-node.steps", "mesh-build-node.toolchains", "mesh-controller.assignments", + "mesh-delivery-gate.source", "mesh-delivery-indexer.candidates", "mesh-delivery-indexer.manifests", "mesh-delivery-indexer.modules" -- 2.54.0 From 8d38c6bac95929794d9b8840cb03bc4ae9a1581e Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 25 Oct 2026 21:16:20 +0100 Subject: [PATCH 3/4] Write the source revision a candidate was previewed onto, so the builder merges its preview onto that trunk head Candidate.Base was never written non-zero (review round 2): the builder's base lookup would have found no trunk for any live candidate and built no preview at all. The indexer now writes, whenever it writes a candidate's fingerprints, the gate's source revision whose trunk heads it read the previews onto; that head of the pull request's repository is the preview's trunk. No mesh-sdk change: the field and its meaning were there. The builder's test now takes the candidate the indexer's own test writes (testdata/candidate-written.json, kept equal to it), not a hand-made one. --- .../internal/build/candidates_test.go | 64 +++++++++++++++++++ .../internal/index/previews.go | 22 +++++-- .../internal/index/previews_test.go | 46 ++++++++++++- .../index/testdata/candidate-written.json | 31 +++++++++ 4 files changed, 157 insertions(+), 6 deletions(-) create mode 100644 modules/mesh-delivery-indexer/internal/index/testdata/candidate-written.json diff --git a/modules/mesh-delivery-builder/internal/build/candidates_test.go b/modules/mesh-delivery-builder/internal/build/candidates_test.go index af8d616..41ca4f1 100644 --- a/modules/mesh-delivery-builder/internal/build/candidates_test.go +++ b/modules/mesh-delivery-builder/internal/build/candidates_test.go @@ -2,9 +2,13 @@ package build import ( "context" + "encoding/json" + "fmt" + "os" "strings" "testing" + "git.novox.be/novox/mesh-sdk/go/catalogue" "git.novox.be/novox/mesh-sdk/go/delivery" ) @@ -47,7 +51,67 @@ const ( headY = "2222222222222222222222222222222222222222" ) +// indexerCandidate is a candidate as the indexer writes it, with the gate's source revision it names (mesh-delivery-indexer +// internal/index testdata/candidate-written.json, kept equal to what the indexer writes by its own test). +func indexerCandidate(t *testing.T) (rev uint64, heads map[string]string, c delivery.Candidate, raw []byte) { + t.Helper() + data, err := os.ReadFile("../../../mesh-delivery-indexer/internal/index/testdata/candidate-written.json") + if err != nil { + t.Fatal(err) + } + var w struct { + Source struct { + Rev uint64 `json:"rev"` + Heads map[string]string `json:"heads"` + } `json:"source"` + Candidate json.RawMessage `json:"candidate"` + } + if err := json.Unmarshal(data, &w); err != nil { + t.Fatal(err) + } + if c, err = delivery.DecodeCandidate(w.Candidate); err != nil { + t.Fatal(err) + } + return w.Source.Rev, w.Source.Heads, c, w.Candidate +} + func TestACandidatesFingerprintIsBuiltOnceAsAKeyOfItsOwnFromTheHeadMergedOntoTheTrunk(t *testing.T) { // N7/AC1, N7/AC2 (issue 1684) + // The candidate the indexer writes is merged onto the trunk head of the source revision it names, never onto the module + // value's commit, which stays at the commit its fingerprint first appeared at (review of the indexer writing on change). + { + rev, heads, c, raw := indexerCandidate(t) + p := c.Pulls[0] + cm := p.Fps["avahi"] + ir := newRig(t, goBundle) + ir.mf.byDigest[cm.Manifest] = catalogue.Manifest{Module: "avahi", Build: &catalogue.Build{Artifacts: []catalogue.Artifact{goBundle}}} + ir.b.Trunk = func(_ context.Context, at uint64, repository string) (string, error) { + if at != rev { + return "", fmt.Errorf("source revision %d is not heard", at) + } + return heads[repository], nil + } + const valueCommit = "7a74cee83f88e6585a5511c3cc53bdf0f5303d20" + value, err := delivery.EncodeModule(delivery.Module{Repository: p.Repository, Commit: valueCommit, FP2: "src2:" + hex64('e'), + Manifest: cm.Manifest, Path: "modules/avahi", Toolchains: agreedToolchains}) + if err != nil { + t.Fatal(err) + } + ir.b.Module(context.Background(), "avahi", value) + ir.b.Wait() + ir.bm.mu.Lock() + ir.bm.calls = nil + ir.bm.mu.Unlock() + ir.b.Candidate(context.Background(), "feat%2Fx", raw) + ir.b.Wait() + fetches := ir.bm.callsOf("build_go_fetch") + if len(fetches) != 1 { + t.Fatalf("%d fetches of the indexer's candidate, want one", len(fetches)) + } + f := fetches[0].body.(map[string]any) + if want := heads[p.Repository]; want == "" || f["base"] != want || f["base"] == valueCommit || f["commit"] != p.Head.Head { + t.Errorf("fetch args = %+v, want the head merged onto the trunk head %.8s of source revision %d", f, want, rev) + } + } r := newRig(t, goBundle) trunkModule(t, r) r.b.Candidate(context.Background(), "feat%2Fx", candidateValue(t, "feat/x", 7, headX)) diff --git a/modules/mesh-delivery-indexer/internal/index/previews.go b/modules/mesh-delivery-indexer/internal/index/previews.go index c2913ea..25b0015 100644 --- a/modules/mesh-delivery-indexer/internal/index/previews.go +++ b/modules/mesh-delivery-indexer/internal/index/previews.go @@ -112,12 +112,12 @@ func (ix *Indexer) RecomputeCandidates(ctx context.Context) error { return nil } -// previewOf is the preview of a head merged onto the trunk head the source names now: found, or stale when only a preview +// previewOf is the preview of a head merged onto the trunk head the source heads name: found, or stale when only a preview // onto another trunk head exists, or neither (found false, stale false) when the source is not known or none exists. -func (ix *Indexer) previewOf(repository, head string) (pv delivery.Preview, found, stale bool) { +func (ix *Indexer) previewOf(heads map[string]string, repository, head string) (pv delivery.Preview, found, stale bool) { ix.mu.Lock() defer ix.mu.Unlock() - trunk := ix.source.heads[repository] + trunk := heads[repository] if trunk == "" { return delivery.Preview{}, false, false } @@ -137,14 +137,23 @@ func (ix *Indexer) previewOf(repository, head string) (pv delivery.Preview, foun // withPreviews gives each touched module of the candidate's pull requests the fingerprint its preview gives it, or the // reason it has none, and the candidate its fpset. It answers whether the candidate changed. A module of a pull request with // no preview keeps what it had. +// +// The candidate's Base is the source revision whose trunk heads the previews were read onto (novox/hq ADR 0437 §5): written +// whenever the fingerprints are, so the builder merges a preview onto the trunk head its fingerprint was computed on (the head +// of the pull request's repository in that revision), never onto a module value's commit, which stays at the commit its +// fingerprint first appeared at. func (ix *Indexer) withPreviews(ctx context.Context, c *delivery.Candidate) bool { - changed := false + ix.mu.Lock() + rev, heads := ix.source.rev, ix.source.heads + ix.mu.Unlock() + changed, read := false, false for i := range c.Pulls { p := &c.Pulls[i] - pv, found, stale := ix.previewOf(p.Repository, p.Head.Head) + pv, found, stale := ix.previewOf(heads, p.Repository, p.Head.Head) if !found && !stale { continue } + read = true // A touched module's directory: the one the modules bucket holds of the repository, else the preview's directory // whose manifest is the one the pull request gave the module (a module the pull request adds; ADR 0501 clause 6). dirs := map[string]string{} @@ -198,6 +207,9 @@ func (ix *Indexer) withPreviews(ctx context.Context, c *delivery.Candidate) bool if changed { c.FPSet = delivery.FingerprintSet(c.Pulls) } + if read && rev != 0 && c.Base != rev && (changed || c.Base == 0) { + c.Base, changed = rev, true + } return changed } diff --git a/modules/mesh-delivery-indexer/internal/index/previews_test.go b/modules/mesh-delivery-indexer/internal/index/previews_test.go index 5d32716..57e7b90 100644 --- a/modules/mesh-delivery-indexer/internal/index/previews_test.go +++ b/modules/mesh-delivery-indexer/internal/index/previews_test.go @@ -2,6 +2,8 @@ package index import ( "context" + "encoding/json" + "os" "strings" "testing" @@ -87,9 +89,16 @@ func TestACandidatesModuleGetsTheFingerprintATrunkModuleOfTheSameTreeHas(t *test if got.FP2 != trunk.FP2 || got.None2 != "" || got.Manifest != delivery.ManifestDigest([]byte(avahiOnTools(t))) { t.Fatalf("candidate avahi = %+v, want the trunk's fingerprint %s", got, trunk.FP2) } - if c := candidateOf(t, cands); c.FPSet != delivery.FingerprintSet(c.Pulls) || c.FPSet == "" { + c := candidateOf(t, cands) + if c.FPSet != delivery.FingerprintSet(c.Pulls) || c.FPSet == "" { t.Errorf("fpset = %q", c.FPSet) } + // The candidate names the source revision the preview was read onto (5, whose catalogue head is the preview's trunk): + // the builder merges the preview onto that head, never onto the module value's commit. + if c.Base != 5 { + t.Errorf("candidate base = %d, want the source revision 5 its preview was read onto", c.Base) + } + written(t, writtenCandidate{Source: sourceRev{Rev: 5, Heads: map[string]string{catalogueRepo: trunkNow}}, Candidate: c}) // Another tree gives another fingerprint. pv := avahiPreview(t, trunkNow, h1) m := pv.Modules["modules/avahi"] @@ -191,3 +200,38 @@ func TestAPreviewOfConflictsGivesTheCandidatesModuleTheReasonItDoesNotMerge(t *t t.Errorf("candidate avahi = %+v, want it not merging and naming the files", got) } } + +// writtenCandidate is what the builder's tests read of the indexer (mesh-delivery-builder internal/build): a candidate as the +// indexer writes it and the gate's source revision it names, kept in testdata/candidate-written.json so the builder is tested +// on what the indexer writes, never on a candidate made by hand. MESH_WRITE_TESTDATA=1 writes it again. +type writtenCandidate struct { + Source sourceRev `json:"source"` + Candidate delivery.Candidate `json:"candidate"` +} + +type sourceRev struct { + Rev uint64 `json:"rev"` + Heads map[string]string `json:"heads"` +} + +func written(t *testing.T, w writtenCandidate) { + t.Helper() + data, err := json.MarshalIndent(w, "", " ") + if err != nil { + t.Fatal(err) + } + data = append(data, '\n') + const file = "testdata/candidate-written.json" + if os.Getenv("MESH_WRITE_TESTDATA") == "1" { + if err := os.WriteFile(file, data, 0o644); err != nil { + t.Fatal(err) + } + } + kept, err := os.ReadFile(file) + if err != nil { + t.Fatal(err) + } + if string(kept) != string(data) { + t.Errorf("%s is not what the indexer writes now (MESH_WRITE_TESTDATA=1 writes it again):\n%s", file, data) + } +} diff --git a/modules/mesh-delivery-indexer/internal/index/testdata/candidate-written.json b/modules/mesh-delivery-indexer/internal/index/testdata/candidate-written.json new file mode 100644 index 0000000..966f47b --- /dev/null +++ b/modules/mesh-delivery-indexer/internal/index/testdata/candidate-written.json @@ -0,0 +1,31 @@ +{ + "source": { + "rev": 5, + "heads": { + "novox/mesh-catalog": "5555555555555555555555555555555555555555" + } + }, + "candidate": { + "v": 1, + "group": "feat/x", + "base": 5, + "heads": "dfe9fb8f79e8", + "pulls": [ + { + "repository": "novox/mesh-catalog", + "number": 301, + "head": "0123456789abcdef0123456789abcdef01234567", + "modules": [ + "avahi" + ], + "fps": { + "avahi": { + "fp2": "src2:e8d595aea1b13f1f5578ef1f245dab0d28a351c28079ecdcd2276c63a932d7be", + "manifest": "8a3856b6c0566d76f2556fc4f46c5251dc7935517bb6bd15809ded7e5546a0d8" + } + } + } + ], + "fpset": "01c840ec276b" + } +} -- 2.54.0 From ac82187b852ce1e2624a8d2c9d0b26da9dfeda82 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 25 Oct 2026 21:28:03 +0100 Subject: [PATCH 4/4] Indexer integration test: a disagreement keeps a computed fingerprint, as issue 1940's fix decided The test predates the fix that keeps a held fingerprint while builders disagree (#47, 3e7f852) and still asserted the blanking that fix removed, so it fails on main 4efe6cf as well. It now asserts the fingerprint is kept after n2's assignment, and that a merge moving avahi during the disagreement gets none until the builders agree. --- .../mesh-delivery-indexer/integration_test.go | 22 ++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) 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 20c9fc1..815e319 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 @@ -642,17 +642,33 @@ func TestIndexerCountsOnlyTheAssignedBuildNodesToolchains(t *testing.T) { return got } agreed := func(m delivery.Module) bool { return m.FP2 != "" && m.Toolchains["go"].Version == "go1.27.3" } - if m := read(agreed); !agreed(m) { - t.Fatalf("within 5 s avahi has fp2 %q, none2 %q, toolchains %v: want a fingerprint of n1's go1.27.3 alone", m.FP2, m.None2, m.Toolchains) + held := read(agreed) + if !agreed(held) { + t.Fatalf("within 5 s avahi has fp2 %q, none2 %q, toolchains %v: want a fingerprint of n1's go1.27.3 alone", held.FP2, held.None2, held.Toolchains) } if _, err := assignments.Put(ctx, "n2.mesh-build-go", []byte(`{"node":"n2","module":"mesh-build-go","assigned":"2026-10-24T08:00:00Z"}`)); err != nil { t.Fatal(err) } + // n2's report, written since its assignment, counts: the builders disagree. A fingerprint already computed is kept + // (issue 1940), and only a new one needs the agreement: a merge moving avahi gets none until they agree. + time.Sleep(500 * time.Millisecond) + if e, err := modules.Get(ctx, "avahi"); err != nil { + t.Fatal(err) + } else if m, err := delivery.DecodeModule(e.Value()); err != nil || m.FP2 != held.FP2 { + t.Fatalf("after n2's assignment avahi has fp2 %q, none2 %q: want its fingerprint %q kept", m.FP2, m.None2, held.FP2) + } + moved := strings.Replace(string(data), `"newtree"`, `"movedtree"`, 1) + if moved == string(data) { + t.Fatal("the merge names no tree to move") + } + if _, err := js.Publish(ctx, "mesh.stream.mesh-delivery-forge-bridge.event.forge.novox.mesh-catalog.merged", []byte(moved)); err != nil { + t.Fatal(err) + } disagree := func(m delivery.Module) bool { return m.FP2 == "" && strings.HasPrefix(m.None2, "builders disagree: go1.27.2 recipe 1 on n2") } if m := read(disagree); !disagree(m) { - t.Fatalf("within 5 s of n2's assignment avahi has fp2 %q, none2 %q: want the builders to disagree", m.FP2, m.None2) + t.Fatalf("within 5 s of a merge after n2's assignment avahi has fp2 %q, none2 %q: want the builders to disagree", m.FP2, m.None2) } if _, err := assignments.Put(ctx, "n2.mesh-build-go", []byte(`{"node":"n2","module":"mesh-build-go","ended":"2026-10-24T09:00:00Z"}`)); err != nil { t.Fatal(err) -- 2.54.0