From 7ba2668aa8620a3884d023ded92fd3ef1c0e660e Mon Sep 17 00:00:00 2001 From: jochen Date: Tue, 3 Nov 2026 16:31:13 +0100 Subject: [PATCH] =?UTF-8?q?The=20check=20refuses=20a=20change=20whose=20mo?= =?UTF-8?q?dule=20graph=20has=20a=20cycle=20and=20names=20it,=20worked=20o?= =?UTF-8?q?ut=20from=20the=20catalogue=20as=20the=20change=20leaves=20it;?= =?UTF-8?q?=20a=20cycle=20the=20trunk=20already=20has=20among=20the=20same?= =?UTF-8?q?=20modules=20is=20said,=20not=20the=20change's=20failure=20(ADR?= =?UTF-8?q?=200603=20=C2=A72)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../cmd/mesh-delivery-checker/judge.go | 39 +++++++++- .../internal/check/examiner_test.go | 28 ++++++++ .../internal/check/graph_cycle.go | 72 +++++++++++++++++++ .../internal/check/judge.go | 5 +- 4 files changed, 142 insertions(+), 2 deletions(-) create mode 100644 modules/mesh-delivery-checker/internal/check/graph_cycle.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 c1fc816e..532f5fff 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge.go @@ -12,6 +12,7 @@ import ( "github.com/nats-io/nats.go/jetstream" "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/delivery" "git.novox.be/novox/mesh-sdk/go/process" @@ -97,7 +98,7 @@ func newJudge(d judgeDeps) check.Judge { nodes = d.nodes } return &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, Announce: d.announce, Trunks: d.trunks, Manifests: d.repos, Predefined: d.predefined, 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, Announce: d.announce, Trunks: d.trunks, Manifests: d.repos, Read: d.repos, Predefined: d.predefined, Logf: d.logf} } // manifestKV reads a manifest by its digest. @@ -122,6 +123,42 @@ type repoModules struct { repoOf map[string]string // by module: its repository commit map[string]string // by module: the commit its record was computed at (the commit the mesh runs) typed map[string]string // by module: its manifest digest + parsed map[string]catalogue.Manifest // by manifest digest: the manifest as read, once (a digest's manifest never changes) +} + +// Manifest is the kept manifest of a digest, read from the indexer's manifests state once and then from memory. +func (r *repoModules) Manifest(ctx context.Context, digest string) (catalogue.Manifest, error) { + r.mu.Lock() + m, known := r.parsed[digest] + r.mu.Unlock() + if known { + return m, nil + } + key, err := delivery.ManifestsKey(digest) + if err != nil { + return catalogue.Manifest{}, err + } + raw, _, found, err := r.manifests.Get(ctx, key) + if err != nil { + return catalogue.Manifest{}, err + } + if !found { + return catalogue.Manifest{}, fmt.Errorf("the indexer keeps no manifest %s", digest) + } + kept, err := delivery.DecodeManifest(raw) + if err != nil { + return catalogue.Manifest{}, err + } + if m, err = parseManifest(kept.JSON); err != nil { + return catalogue.Manifest{}, err + } + r.mu.Lock() + if r.parsed == nil { + r.parsed = map[string]catalogue.Manifest{} + } + r.parsed[digest] = m + r.mu.Unlock() + return m, nil } // Put takes a change of the indexer's modules bucket in: a module's repository and the languages its manifest names. diff --git a/modules/mesh-delivery-checker/internal/check/examiner_test.go b/modules/mesh-delivery-checker/internal/check/examiner_test.go index 6e5b4734..7da5b258 100644 --- a/modules/mesh-delivery-checker/internal/check/examiner_test.go +++ b/modules/mesh-delivery-checker/internal/check/examiner_test.go @@ -20,6 +20,7 @@ import ( gobuild "git.novox.be/novox/mesh-catalog/modules/mesh-build-go/buildstep" "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" "git.novox.be/novox/mesh-sdk/go/delivery" ) @@ -1000,6 +1001,17 @@ func (k keptList) PredefinedList(context.Context) (json.RawMessage, error) { return json.RawMessage(k), nil } +// fixedManifests are the indexer's kept manifests by digest as a test gives them. +type fixedManifests map[string]catalogue.Manifest + +func (f fixedManifests) Manifest(_ context.Context, digest string) (catalogue.Manifest, error) { + m, ok := f[digest] + if !ok { + return catalogue.Manifest{}, errors.New("no manifest " + digest) + } + return m, nil +} + // fixedDigests are the indexer's manifest digests as a test gives them. type fixedDigests map[string]string @@ -1017,17 +1029,33 @@ func TestTheIndexersJudgeJudgesTheChangesManifests(t *testing.T) { // untouched is the judge's refusal of mail, which the change leaves as it is, and trunkMail its refusal on the trunk: // a problem the trunk had is a warning, one the change causes fails (ADR 0579 §2) untouched, trunkMail string + // cycle reads the manifests, so the change's module graph is worked out (ADR 0603 §2): the change's web awaits + // mail's site while mail awaits web's database; trunkCycle has the trunk's web await it too. + cycle, trunkCycle bool }{ {name: "accepted"}, {name: "an untouched manifest's problem the trunk had", untouched: "mail says no credential form", trunkMail: "mail says no credential form"}, {name: "a problem the change causes in an untouched manifest", untouched: "mail reads a state web no longer keeps", fails: "refuses the manifest of mail: mail reads a state web no longer keeps"}, + {name: "a cycle the change makes", cycle: true, fails: "the change's module graph has the cycle mail -> web -> mail"}, + {name: "a cycle the trunk has", cycle: true, trunkCycle: true}, {name: "refused", refused: "web requires \"amqp\"\nweb has no id", fails: "the indexer's judge at mesh-delivery's trunk " + strings.Repeat("d", 40) + ` refuses the manifest of web: web requires "amqp"; web has no id`}, } { b := newBench(t) b.ex.Manifests = fixedDigests{"mail": strings.Repeat("a", 64), "web": strings.Repeat("b", 64)} b.ex.Trunks = fixedTrunks{heads: map[string]string{DeliveryRepository: trunk}} b.ex.Predefined = keptList(`{"font-size": {"type": "number", "description": "the size"}}`) + if row.cycle { + offer := func(name string) []catalogue.Offer { + return []catalogue.Offer{{Name: name, Scope: catalogue.ScopeMesh, AnsweredBy: catalogue.AnsweredByHelper}} + } + trunkWeb := catalogue.Manifest{Module: "web", Provides: offer("database")} + if row.trunkCycle { + trunkWeb.Requires = []string{"site"} + } + b.ex.Read = fixedManifests{strings.Repeat("a", 64): {Module: "mail", Provides: offer("site"), Requires: []string{"database"}}, + strings.Repeat("b", 64): trunkWeb, web: {Module: "web", Provides: offer("database"), Requires: []string{"site"}}} + } var judged []delivery.ComposeJob answer := b.asks.Answer b.asks.Answer = func(a delivery.StepAsk) delivery.Step { diff --git a/modules/mesh-delivery-checker/internal/check/graph_cycle.go b/modules/mesh-delivery-checker/internal/check/graph_cycle.go new file mode 100644 index 00000000..0490f9a9 --- /dev/null +++ b/modules/mesh-delivery-checker/internal/check/graph_cycle.go @@ -0,0 +1,72 @@ +package check + +import ( + "context" + "fmt" + "maps" + "slices" + "strings" + + "git.novox.be/novox/mesh-sdk/go/catalogue" + "git.novox.be/novox/mesh-sdk/go/composer" + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +// The check refuses a change whose module graph has a cycle and names it (novox/hq ADR 0603 §2): modules that would each +// wait for the other for good, through their bases or the provisions they await. The graph is worked out from the +// catalogue as the change leaves it (every trunk manifest, each one a pull request touches replaced by its candidate's), +// with the contract's ManifestGraph, the way the indexer works out its graph's edges. A cycle the trunk already has among +// the same modules is the trunk's: said in the verdict's notes, not the change's failure. + +// ManifestReader reads a kept manifest by its digest (the indexer's manifests state). +type ManifestReader interface { + Manifest(ctx context.Context, digest string) (catalogue.Manifest, error) +} + +// refuseCycle fails the check for a cycle of the change's module graph, naming it; nil when the case touches no manifest, +// when nothing reads manifests, or when there is none. A manifest that cannot be read fails the check, said, never passes it. +func (st *state) refuseCycle(ctx context.Context) *delivery.Verdict { + if st.j.Manifests == nil || st.j.Read == nil || st.c.Try != nil { + return nil + } + trunk := st.j.Manifests.ManifestDigests() + req, touched := judgeRequest(trunk, st.c.Pulls) + if len(touched) == 0 { + return nil + } + read := func(digests map[string]composer.Manifest) (map[string]catalogue.Manifest, error) { + out := map[string]catalogue.Manifest{} + for _, module := range slices.Sorted(maps.Keys(digests)) { + m, err := st.j.Read.Manifest(ctx, digests[module].Digest) + if err != nil { + return nil, fmt.Errorf("the manifest %s of %s: %w", digests[module].Digest, module, err) + } + out[module] = m + } + return out, nil + } + change, err := read(req.Cases[0].Manifests) + if err != nil { + return fail(goModule, "the change's module graph cannot be worked out, so no cycle can be ruled out: "+err.Error()) + } + cycle := delivery.ManifestGraph(change).Cycle() + if cycle == nil { + st.c.note("the change's module graph has no cycle") + return nil + } + words := strings.Join(cycle, " -> ") + if before, err := read(req.Cases[1].Manifests); err == nil { + among := delivery.Graph{} + for name, n := range delivery.ManifestGraph(before) { + if slices.Contains(cycle, name) { + among[name] = n + } + } + if among.Cycle() != nil { + st.c.note("the module graph has the cycle " + words + ", which the trunk already has among the same modules (ADR 0603 §2)") + return nil + } + } + return fail(goModule, fmt.Sprintf("the change's module graph has the cycle %s: these modules would each wait for the other for "+ + "good, through their bases or the provisions they await (ADR 0603 §2)", words)) +} diff --git a/modules/mesh-delivery-checker/internal/check/judge.go b/modules/mesh-delivery-checker/internal/check/judge.go index b561a1fb..9d7a2283 100644 --- a/modules/mesh-delivery-checker/internal/check/judge.go +++ b/modules/mesh-delivery-checker/internal/check/judge.go @@ -114,6 +114,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 + // 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 // Predefined reads the catalogue's predefined keys the indexer keeps (manifests_judge.go); nil reads none. Predefined PredefinedList // Compositions makes a check's composition (ADR 0593 §7): the engines' answers (EngineCompositions); nil leaves the @@ -242,7 +245,7 @@ func (j *Examiner) Judge(ctx context.Context, c *Case) (Outcome, error) { // Every member and level runs to its end, and every manifest the change touches is judged by the indexer's judge (ADR // 0578 §2), also after a failure: the verdict names every failure, not only the first (the operator, 2026-11-02). tests := st.judgeAll(ctx) - if v := settled(joinFailures(tests, st.judgeManifests(ctx))); v != nil { + if v := settled(joinFailures(tests, st.judgeManifests(ctx), st.refuseCycle(ctx))); v != nil { return st.finish(ctx, *v), nil } return st.finish(ctx, delivery.Verdict{State: delivery.VerdictPass}), nil