From 30ee47caa608eaf421b21040145f3eb8c433cb11 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 4 Nov 2026 14:32:41 +0100 Subject: [PATCH] DLV-C5: the checker asks the seat mesh-config's verb renames for every change that brings or drops a renamed-from Design 32 (A config value renamed): the checker invokes seat:mesh-config.renames, never the module. A change bringing renamed-from to a holder that serves no renames (nothing answers the optional verb) is refused naming it; dropping a renamed-from while its window is open is refused naming the nodes holding it open; a check that cannot ask fails, saying so. The pull request's trunk and head manifests are read once for this and for the series changes (manifestPair). Rows in TestTheIndexersJudgeJudgesTheChangesManifests with the verb faked from the seat's definition (its fixture's answer): not served, served, open on g14, closed, an ask that fails, no seat to ask; the four refusals failed first. Manifest row for the invokes. Claude-Session: https://claude.ai/code/session_01JG73KJNrH5EgFXMKKYSrig --- .../cmd/mesh-delivery-checker/judge.go | 3 +- .../cmd/mesh-delivery-checker/main.go | 1 + .../cmd/mesh-delivery-checker/renames.go | 36 +++++ modules/mesh-delivery-checker/go.mod | 1 + .../internal/check/examiner_test.go | 42 ++++++ .../internal/check/judge.go | 5 +- .../internal/check/renames.go | 127 ++++++++++++++++++ .../internal/check/series.go | 67 ++++----- .../mesh-delivery-checker/manifest_test.go | 2 +- modules/mesh-delivery-checker/module.json | 1 + 10 files changed, 250 insertions(+), 35 deletions(-) create mode 100644 modules/mesh-delivery-checker/cmd/mesh-delivery-checker/renames.go create mode 100644 modules/mesh-delivery-checker/internal/check/renames.go 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 acd86ce7..bbeef436 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge.go @@ -25,6 +25,7 @@ import ( // judgeDeps are the parts the judge of a check is made of. type judgeDeps struct { asks check.Asks + renames check.Renames tools *buildrun.ToolchainSet trunks *check.Trunks kept check.KV @@ -155,7 +156,7 @@ func newJudge(d judgeDeps) check.Judge { nodes = d.nodes } ex := &check.Examiner{Nodes: nodes, Compositions: d.compositions, Asks: d.asks, Toolchains: d.tools, - Kept: check.Kept{Store: d.kept}, Previews: d.previews, Modules: d.repos, Packages: d.repos, Published: d.published, Forge: d.forge, Runs: d.repos, Builds: d.builds, BuildEnds: d.ends, BuildWait: d.buildWait, Announce: d.announce, Trunks: d.trunks, Manifests: d.repos, Read: d.repos, Dirs: d.repos, Predefined: d.predefined, Tags: d.tags, Logf: d.logf} + Kept: check.Kept{Store: d.kept}, Previews: d.previews, Modules: d.repos, Packages: d.repos, Published: d.published, Forge: d.forge, Runs: d.repos, Builds: d.builds, BuildEnds: d.ends, BuildWait: d.buildWait, Announce: d.announce, Trunks: d.trunks, Manifests: d.repos, Read: d.repos, Dirs: d.repos, Predefined: d.predefined, Tags: d.tags, Renames: d.renames, Logf: d.logf} if d.nodes != nil && d.builds != nil && d.repos != nil { ex.Serving = runningServes{have: d.nodes, builds: d.builds, read: d.repos, logf: d.logf} } diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go index eda2e94f..9f216641 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go @@ -263,6 +263,7 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro announce: announce, compositions: compositions, tags: tags, nodes: openNodeNames(ctx, js, logger.Printf), predefined: keptPredefined{predefined}, + renames: seatRenames{call: caller.Call}, }) ch.Announce = announce tryer.Judge = ch.Judge diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/renames.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/renames.go new file mode 100644 index 00000000..dca90323 --- /dev/null +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/renames.go @@ -0,0 +1,36 @@ +package main + +import ( + "context" + "encoding/json" + "errors" + "fmt" + + "github.com/nats-io/nats.go" + + "git.novox.be/novox/mesh-contracts/go/seats/meshconfig" + + "mesh-delivery-checker/internal/check" +) + +// seatRenames asks the seat mesh-config's verb renames as the checker's own account (its `invokes` +// seat:mesh-config.renames; design 32). A call nobody answers is a holder serving no renames (the verb is optional), said +// as check.ErrRenamesNotServed with the bus's words; any other error is an ask that could not be made. +type seatRenames struct { + call func(key string, body any) (json.RawMessage, error) +} + +func (s seatRenames) Renames(_ context.Context, module string) (meshconfig.RenamesAnswer, error) { + raw, err := s.call(check.RenamesSeat, meshconfig.RenamesInput{Module: module}) + if errors.Is(err, nats.ErrNoResponders) { + return meshconfig.RenamesAnswer{}, fmt.Errorf("%w (nothing answers it: %v)", check.ErrRenamesNotServed, err) + } + if err != nil { + return meshconfig.RenamesAnswer{}, err + } + var a meshconfig.RenamesAnswer + if err := json.Unmarshal(raw, &a); err != nil { + return meshconfig.RenamesAnswer{}, fmt.Errorf("%s answered what is not its answer: %w", check.RenamesSeat, err) + } + return a, nil +} diff --git a/modules/mesh-delivery-checker/go.mod b/modules/mesh-delivery-checker/go.mod index 929f8473..8781e9a9 100644 --- a/modules/mesh-delivery-checker/go.mod +++ b/modules/mesh-delivery-checker/go.mod @@ -12,6 +12,7 @@ require git.novox.be/novox/mesh-catalog/modules/mesh-build-go/buildstep v0.0.0-2 require ( git.novox.be/novox/mesh-contracts/go/seats/gopackageregistry v0.0.0-00010101000000-000000000000 + git.novox.be/novox/mesh-contracts/go/seats/meshconfig v0.0.0-00010101000000-000000000000 git.novox.be/novox/mesh-contracts/go/seats/meshdeliverychecker v0.0.0-00010101000000-000000000000 git.novox.be/novox/mesh-contracts/go/seats/npmpackageregistry v0.0.0-00010101000000-000000000000 git.novox.be/novox/mesh-delivery/modules/mesh-delivery-builder/buildrun v0.0.0-00010101000000-000000000000 diff --git a/modules/mesh-delivery-checker/internal/check/examiner_test.go b/modules/mesh-delivery-checker/internal/check/examiner_test.go index ecda3707..264cf0d8 100644 --- a/modules/mesh-delivery-checker/internal/check/examiner_test.go +++ b/modules/mesh-delivery-checker/internal/check/examiner_test.go @@ -19,6 +19,7 @@ import ( "unicode/utf8" gobuild "git.novox.be/novox/mesh-catalog/modules/mesh-build-go/buildstep" + "git.novox.be/novox/mesh-contracts/go/seats/meshconfig" "git.novox.be/novox/mesh-delivery/modules/mesh-delivery-builder/buildrun" "git.novox.be/novox/mesh-sdk/go/catalogue" "git.novox.be/novox/mesh-sdk/go/composer" @@ -1037,7 +1038,18 @@ func TestTheIndexersJudgeJudgesTheChangesManifests(t *testing.T) { trunkCycle bool // series is the series of web's package the change raises it to, and the words its status line names (DLV-C4). series, seriesWords string + // renames is what the change does to web's renamed-from ("bring", "drop") and what the seat mesh-config's verb + // renames answers ("served", "open" on g14, "not served", "unasked" with no seat to ask, "error"); DLV-C5. + renames, seat string }{ + {name: "a rename brought, the verb not served", renames: "bring", seat: "not served", + fails: "web (novox/mesh-delivery#7) renames new-key from old-key: refused, the holder of the seat mesh-config does not serve its verb renames"}, + {name: "a rename brought, the verb served", renames: "bring", seat: "served"}, + {name: "a renamed-from dropped while its window is open", renames: "drop", seat: "open", + fails: "web (novox/mesh-delivery#7) drops renamed-from old-key of new-key while its rename window is open on g14"}, + {name: "a renamed-from dropped once its window closed", renames: "drop", seat: "served"}, + {name: "a rename the check cannot ask of", renames: "bring", seat: "error", fails: "the check cannot ask seat:mesh-config.renames: the bus said no"}, + {name: "a rename with no seat to ask", renames: "drop", seat: "unasked", fails: "this checker cannot ask seat:mesh-config.renames"}, {name: "accepted"}, {name: "a package series raised", series: "0.2", seriesWords: "web/pkg series 0.1 → 0.2"}, {name: "a package series kept", series: "0.1"}, @@ -1088,6 +1100,28 @@ func TestTheIndexersJudgeJudgesTheChangesManifests(t *testing.T) { b.ex.Read = fixedManifests{strings.Repeat("a", 64): {Module: "mail"}, strings.Repeat("b", 64): {Module: "web", Build: pkg("")}, web: {Module: "web", Build: pkg(row.series)}} } + if row.renames != "" { + renamed := map[string]catalogue.SettingDeclaration{"new-key": {Type: catalogue.ConfigString, RenamedFrom: "old-key"}} + plain := map[string]catalogue.SettingDeclaration{"new-key": {Type: catalogue.ConfigString}} + trunkWeb, headWeb := catalogue.Manifest{Module: "web", Settings: plain}, catalogue.Manifest{Module: "web", Settings: renamed} + if row.renames == "drop" { + trunkWeb, headWeb = headWeb, trunkWeb + } + b.ex.Read = fixedManifests{strings.Repeat("a", 64): {Module: "mail"}, strings.Repeat("b", 64): trunkWeb, web: headWeb} + // The verb faked from its definition: the answer of the seat's fixture (mesh-contracts seats/mesh-config). + fake := fakeRenames{} + switch row.seat { + case "not served": + fake.err = fmt.Errorf("%w (nothing answers it)", ErrRenamesNotServed) + case "error": + fake.err = errors.New("the bus said no") + case "open": + fake.answer = meshconfig.RenamesAnswer{Renames: []meshconfig.RenamesAnswerRenamesItem{{Module: "web", From: "old-key", To: "new-key", At: "2026-11-04T12:00:00Z", Open: []string{"g14"}}}} + } + if row.seat != "unasked" { + b.ex.Renames = fake + } + } var judged []delivery.ComposeJob answer := b.asks.Answer b.asks.Answer = func(a delivery.StepAsk) delivery.Step { @@ -1134,3 +1168,11 @@ func TestTheIndexersJudgeJudgesTheChangesManifests(t *testing.T) { t.Error("which merge changes the judge") } } + +// fakeRenames is the seat mesh-config's verb renames as its definition answers it. +type fakeRenames struct { + answer meshconfig.RenamesAnswer + err error +} + +func (f fakeRenames) Renames(context.Context, string) (meshconfig.RenamesAnswer, error) { return f.answer, f.err } diff --git a/modules/mesh-delivery-checker/internal/check/judge.go b/modules/mesh-delivery-checker/internal/check/judge.go index 70ebb497..a8e98824 100644 --- a/modules/mesh-delivery-checker/internal/check/judge.go +++ b/modules/mesh-delivery-checker/internal/check/judge.go @@ -128,6 +128,9 @@ type Examiner struct { // Manifests are the indexer's manifest digests by module: the catalogue a change's manifests are judged in by the // indexer's judge (manifests_judge.go, novox/hq ADR 0578 §2); nil judges no manifest. Manifests TrunkManifests + // Renames asks the seat mesh-config's verb renames for a change that brings or drops a `renamed-from` (renames.go, + // design 32); nil fails such a change, saying it cannot ask. + Renames Renames // Read reads a kept manifest by its digest: the change's module graph is worked out from them (graph_cycle.go, novox/hq // ADR 0603 §2); nil works out no graph. Read ManifestReader @@ -270,7 +273,7 @@ func (j *Examiner) Judge(ctx context.Context, c *Case) (Outcome, error) { // 0578 §2), also after a failure: the verdict names every failure, not only the first (the operator, 2026-11-02). tests := st.judgeAll(ctx) st.seriesChanges(ctx) // DLV-C4: named in the verdict's line, whatever the verdict - if v := settled(joinFailures(tests, st.judgeManifests(ctx), st.refuseCycle(ctx))); v != nil { + if v := settled(joinFailures(tests, st.judgeManifests(ctx), st.refuseCycle(ctx), st.judgeRenames(ctx))); v != nil { return st.finish(ctx, *v), nil } return st.finish(ctx, delivery.Verdict{State: delivery.VerdictPass}), nil diff --git a/modules/mesh-delivery-checker/internal/check/renames.go b/modules/mesh-delivery-checker/internal/check/renames.go new file mode 100644 index 00000000..0d8bfc67 --- /dev/null +++ b/modules/mesh-delivery-checker/internal/check/renames.go @@ -0,0 +1,127 @@ +package check + +import ( + "context" + "errors" + "fmt" + "maps" + "slices" + "strings" + + "git.novox.be/novox/mesh-contracts/go/seats/meshconfig" + "git.novox.be/novox/mesh-sdk/go/catalogue" + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +// A config value renamed (design 32 *A config value renamed*, DLV-C5): the checker asks the seat mesh-config's verb +// renames, never the module, for every change that brings `renamed-from` or drops one. A holder that does not serve the +// verb refuses `renamed-from`, naming that; while a rename's window is open, dropping its `renamed-from` is refused, +// naming the nodes holding it open; a check that cannot ask fails, saying so. + +// RenamesSeat is the verb the checker asks, as its manifest's `invokes` names it. +var RenamesSeat = "seat:" + meshconfig.Seat + "." + meshconfig.VerbRenames + +// ErrRenamesNotServed is the answer of Renames when the seat's holder serves no verb renames. +var ErrRenamesNotServed = errors.New("the holder of the seat " + meshconfig.Seat + " does not serve its verb " + meshconfig.VerbRenames) + +// Renames asks the seat mesh-config's verb renames for a module's open renames. +type Renames interface { + Renames(ctx context.Context, module string) (meshconfig.RenamesAnswer, error) +} + +// renameChange is what a pull request's head does to a module's `renamed-from` against the trunk: the declarations it +// brings (key from old name) and the ones it drops (key, old name). +type renameChange struct { + brought, dropped map[string]string +} + +// renamesOf compares a module's config declarations at the trunk and at the head. +func renamesOf(was, now catalogue.Manifest) renameChange { + rc := renameChange{brought: map[string]string{}, dropped: map[string]string{}} + for key, d := range now.Settings { + if d.RenamedFrom != "" && was.Settings[key].RenamedFrom != d.RenamedFrom { + rc.brought[key] = d.RenamedFrom + } + } + for key, d := range was.Settings { + if d.RenamedFrom != "" && now.Settings[key].RenamedFrom != d.RenamedFrom { + rc.dropped[key] = d.RenamedFrom + } + } + return rc +} + +// judgeRenames fails the check for a rename the seat refuses, a window a change would drop, or an ask that cannot be made. +func (st *state) judgeRenames(ctx context.Context) *delivery.Verdict { + if st.c.Try != nil { + return nil + } + var out *delivery.Verdict + failed := func(words string) { + f := delivery.FailedFirst{Machine: meshconfig.Seat, Words: words} + if out == nil { + out = &delivery.Verdict{State: delivery.VerdictFail, Failed: f} + return + } + addFailure(out, f) + } + for _, p := range st.c.Pulls { + for _, module := range p.Modules { + was, now, ok, err := st.manifestPair(ctx, p, module) + if err != nil { + failed(fmt.Sprintf("%s (%s): its config renames cannot be judged: %v", module, pullKey(p), err)) + continue + } + if !ok { + was = catalogue.Manifest{} // a module new on the pull request brings every rename it declares + if now, ok = st.headManifest(ctx, p, module); !ok { + continue + } + } + rc := renamesOf(was, now) + if len(rc.brought) == 0 && len(rc.dropped) == 0 { + continue + } + if st.j.Renames == nil { + failed(fmt.Sprintf("%s (%s) changes a renamed-from, and this checker cannot ask %s", module, pullKey(p), RenamesSeat)) + continue + } + answer, err := st.j.Renames.Renames(ctx, module) + switch { + case errors.Is(err, ErrRenamesNotServed): + for _, key := range slices.Sorted(maps.Keys(rc.brought)) { + failed(fmt.Sprintf("%s (%s) renames %s from %s: refused, %v, so the value cannot be moved", module, pullKey(p), key, rc.brought[key], err)) + } + for _, key := range slices.Sorted(maps.Keys(rc.dropped)) { + failed(fmt.Sprintf("%s (%s) drops renamed-from %s of %s: refused, %v, so whether its window is open is not known", module, pullKey(p), rc.dropped[key], key, err)) + } + continue + case err != nil: + failed(fmt.Sprintf("%s (%s) changes a renamed-from, and the check cannot ask %s: %v", module, pullKey(p), RenamesSeat, err)) + continue + } + for _, key := range slices.Sorted(maps.Keys(rc.dropped)) { + for _, r := range answer.Renames { + if r.Module == module && r.From == rc.dropped[key] && r.To == key && len(r.Open) > 0 { + open := slices.Sorted(slices.Values(r.Open)) + failed(fmt.Sprintf("%s (%s) drops renamed-from %s of %s while its rename window is open on %s", module, pullKey(p), r.From, key, strings.Join(open, ", "))) + } + } + } + st.c.note(fmt.Sprintf("%s.%s answered for %s (%s)", meshconfig.Seat, meshconfig.VerbRenames, module, pullKey(p))) + } + } + return out +} + +// headManifest is a module's manifest at the pull request's head, new on it: by its candidate's digest, else the forge's. +func (st *state) headManifest(ctx context.Context, p Pull, module string) (catalogue.Manifest, bool) { + if digest := p.Fps[module].Manifest; digest != "" && st.j.Read != nil { + if st.j.Manifests != nil && st.j.Manifests.ManifestDigests()[module] == digest { + return catalogue.Manifest{}, false // unchanged + } + m, err := st.j.Read.Manifest(ctx, digest) + return m, err == nil + } + return catalogue.Manifest{}, false +} diff --git a/modules/mesh-delivery-checker/internal/check/series.go b/modules/mesh-delivery-checker/internal/check/series.go index 786d90bb..a2e00760 100644 --- a/modules/mesh-delivery-checker/internal/check/series.go +++ b/modules/mesh-delivery-checker/internal/check/series.go @@ -16,45 +16,17 @@ import ( // seriesChanges names, on the case per pull request, each change of an artifact's series the pull request makes. func (st *state) seriesChanges(ctx context.Context) { - if st.j.Read == nil { - return - } - var trunk map[string]string - if st.j.Manifests != nil { - trunk = st.j.Manifests.ManifestDigests() - } for _, p := range st.c.Pulls { var words []string for _, module := range p.Modules { - if trunk[module] == "" { - continue // new on this pull request: no series is changed - } - was, err := st.j.Read.Manifest(ctx, trunk[module]) + was, now, ok, err := st.manifestPair(ctx, p, module) if err != nil { - words = append(words, fmt.Sprintf("%s: the trunk's series not read (%v)", module, err)) + words = append(words, fmt.Sprintf("%s: its series not compared (%v)", module, err)) continue } - var now catalogue.Manifest - switch digest := p.Fps[module].Manifest; { - case digest == trunk[module]: - continue - case digest != "": - if now, err = st.j.Read.Manifest(ctx, digest); err != nil { - words = append(words, fmt.Sprintf("%s: the head's series not read (%v)", module, err)) - continue - } - case st.j.Forge != nil: - var found bool - if now, _, found, err = st.manifestAtHead(ctx, p, module); err != nil { - words = append(words, fmt.Sprintf("%s: the head's series not read (%v)", module, err)) - continue - } else if !found { - continue - } - default: - continue + if ok { + words = append(words, seriesMoved(module, was, now)...) } - words = append(words, seriesMoved(module, was, now)...) } if len(words) > 0 { st.c.series(pullKey(p), words) @@ -62,6 +34,37 @@ func (st *state) seriesChanges(ctx context.Context) { } } +// manifestPair is a touched module's trunk manifest and its manifest at the pull request's head: the head's by the digest +// its candidate names, else read from the forge at the head. ok is false when the module is new on the pull request, +// unchanged, or not read here. +func (st *state) manifestPair(ctx context.Context, p Pull, module string) (was, now catalogue.Manifest, ok bool, err error) { + if st.j.Read == nil || st.j.Manifests == nil { + return was, now, false, nil + } + trunk := st.j.Manifests.ManifestDigests()[module] + digest := p.Fps[module].Manifest + if trunk == "" || digest == trunk { + return was, now, false, nil + } + if was, err = st.j.Read.Manifest(ctx, trunk); err != nil { + return was, now, false, fmt.Errorf("the trunk's manifest not read: %w", err) + } + switch { + case digest != "": + if now, err = st.j.Read.Manifest(ctx, digest); err != nil { + return was, now, false, fmt.Errorf("the head's manifest not read: %w", err) + } + case st.j.Forge != nil: + var found bool + if now, _, found, err = st.manifestAtHead(ctx, p, module); err != nil || !found { + return was, now, false, err + } + default: + return was, now, false, nil + } + return was, now, true, nil +} + // seriesMoved are the artifacts of module whose series the head's manifest changes from the trunk's. func seriesMoved(module string, was, now catalogue.Manifest) []string { if was.Build == nil || now.Build == nil { diff --git a/modules/mesh-delivery-checker/manifest_test.go b/modules/mesh-delivery-checker/manifest_test.go index 41491b4e..97565506 100644 --- a/modules/mesh-delivery-checker/manifest_test.go +++ b/modules/mesh-delivery-checker/manifest_test.go @@ -123,7 +123,7 @@ func TestManifestDeclarations(t *testing.T) { // 1678/AC1 if want := []string{"gitea.gitea_commits", "gitea.gitea_compare", "gitea.gitea_get_file", "gitea.gitea_list_branches", "mesh-delivery-assigner.assigner_read_placement", "mesh-delivery-indexer.indexer_read_candidates", "mesh-delivery-indexer.indexer_read_manifests", "mesh-delivery-indexer.indexer_read_modules", "mesh-delivery-indexer.indexer_read_packages", "mesh-delivery-indexer.indexer_read_predefined", - "seat:node-engine.compose-candidate"}; !reflect.DeepEqual(sorted(m.Invokes), want) { + "seat:mesh-config.renames", "seat:node-engine.compose-candidate"}; !reflect.DeepEqual(sorted(m.Invokes), want) { t.Errorf("invokes = %v, want the forge's reads and compose-candidate", m.Invokes) } if want := []string{"node-build-go", "node-build-node"}; !reflect.DeepEqual(sorted(m.Uses), want) { diff --git a/modules/mesh-delivery-checker/module.json b/modules/mesh-delivery-checker/module.json index df8cfd0c..759ea326 100644 --- a/modules/mesh-delivery-checker/module.json +++ b/modules/mesh-delivery-checker/module.json @@ -79,6 +79,7 @@ "mesh-delivery-indexer.indexer_read_modules", "mesh-delivery-indexer.indexer_read_packages", "mesh-delivery-indexer.indexer_read_predefined", + "seat:mesh-config.renames", "seat:node-engine.compose-candidate" ],