From 53addd73c358daae84e24da7e5cd9227b9901879 Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 7 Nov 2026 07:24:16 +0100 Subject: [PATCH] The check's composer composes a module at its build only where that build was made of the change's manifest (issue 1921, milestone 47/G) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The running indexer could not read 17 of the group's manifests, so the running node-engine composed those modules at their trunk builds and said nothing; the head composer laid those builds over the change's other manifests and ace failed on "nothing provides artifact-store, wanted by mesh-build-go" (group check 8a80e8e86858). The input a node answers a check with now names, per module taken in from its build, the content digest of the manifest that build was made of (ComposedHow.Manifests); the composer compares it with the case's manifest, and, for an input from a node-engine before it, compares what the two manifests say beyond what a take-in resolves (the lead's ruling, §17). Each module lifted either way is composed from the case's manifest, its build awaited, and named in NotJudged with the comparison that lifted it; one that cannot compose without its build is held as its stub. The digest is taken over one JSON form because a request through a build module arrives compacted. Claude-Session: https://claude.ai/code/session_01NcbgfFSnn24M3GcqttoU7i --- internal/compose/frombus/answer_rows_test.go | 10 +++ internal/compose/frombus/frombus.go | 15 +++- internal/compose/input.go | 25 ++++++ internal/composer/composer.go | 89 +++++++++++++++++++- internal/composer/composer_test.go | 49 +++++++++++ 5 files changed, 185 insertions(+), 3 deletions(-) diff --git a/internal/compose/frombus/answer_rows_test.go b/internal/compose/frombus/answer_rows_test.go index 15c33bc..eeb0894 100644 --- a/internal/compose/frombus/answer_rows_test.go +++ b/internal/compose/frombus/answer_rows_test.go @@ -1,6 +1,7 @@ package frombus import ( + "bytes" "context" "encoding/json" "fmt" @@ -218,6 +219,15 @@ func answerNamesEveryRefusalAndLeavesAnOldBuildToTheNode(t *testing.T) { t.Errorf("spiegel in InOwn wanted at %s: %v, held %v, read as %s; want its build's image", sp.Fp2, err, read.ComposedHow, raw) } + // Issue 1921: the input a check is answered with names the manifest spiegel's build was made of (its content + // digest, the same for those bytes compacted inside a request), so the check's composer composes it at that + // build only where the change's manifest is that one. + var compact bytes.Buffer + _ = json.Compact(&compact, bb.manifests[sp.Manifest]) + want, _ := compose.ContentDigest(bb.manifests[sp.Manifest]) + if got, _ := compose.ContentDigest(compact.Bytes()); err == nil && (want == "" || read.ComposedHow.Manifests["spiegel"] != want || got != want) { + t.Errorf("the manifest spiegel's build was made of: %v, want %s (compacted %s)", read.ComposedHow.Manifests, want, got) + } // Issue 1921: held by its manifest's verdict, spiegel is still held in what the node composes, and the input a check // is answered with says what holds it and carries the build the node would compose it at, taken in while done; a // node's own read takes in no build of it. diff --git a/internal/compose/frombus/frombus.go b/internal/compose/frombus/frombus.go index 18e45a0..58bda5b 100644 --- a/internal/compose/frombus/frombus.go +++ b/internal/compose/frombus/frombus.go @@ -171,6 +171,9 @@ type Scope struct { // on the node that its manifest's verdict or its decoder holds, into compose.Input.Holds, never into what the node composes: the // check's composer, judging the change's manifest itself, composes it at that build (issue 1921). takeInHeld bool + // manifests, set with takeInHeld, is filled with the ContentDigest of the manifest each module placed on the node was + // built from, as it is taken in from that build (compose.ComposedHow.Manifests). + manifests map[string]string // Holds is told, after Compose read the input, what holds each module it holds by its manifest's verdict or its // decoder (compose.Input.Holds), with the build it would compose it at, so the node says why it holds it (issue 1921, // the composition verb). Nil: said nowhere. @@ -624,12 +627,12 @@ func Compose(ctx context.Context, r Reader, mesh Mesh, s Scope, fps map[string]s // credential: the sandbox composes no pair value. func ComposingInput(ctx context.Context, r Reader, mesh Mesh, s Scope, fps map[string]string) (compose.Input, error) { held := map[string]string{} - s.takeInHeld = true + s.takeInHeld, s.manifests = true, map[string]string{} in, err := input(ctx, r, mesh, s, func(module string, err error) { held[module] = err.Error() }, fps) if err != nil { return compose.Input{}, err } - c := &compose.ComposedHow{Node: s.Node, Builds: maps.Clone(fps), Held: held} + c := &compose.ComposedHow{Node: s.Node, Builds: maps.Clone(fps), Held: held, Manifests: s.manifests} for _, run := range s.Running { if _, isHeld := held[run.Module]; isHeld && run.Module != "" { c.Running = append(c.Running, run) @@ -983,6 +986,14 @@ func fromBuilds(ctx context.Context, r Reader, build func(ctx context.Context, m continue } in.Catalogue[pm.Module] = resolved + if s.manifests != nil { + // Unread or not JSON, it is left out: the check's composer then compares what the manifests say (issue 1921). + if raw, err := r.Manifest(ctx, b.Manifest); err == nil { + if digest, err := compose.ContentDigest(raw); err == nil { + s.manifests[pm.Module] = digest + } + } + } } for i := range in.Nodes { if in.Nodes[i].Name != s.Node { diff --git a/internal/compose/input.go b/internal/compose/input.go index 74c7c8d..7c1de99 100644 --- a/internal/compose/input.go +++ b/internal/compose/input.go @@ -16,6 +16,9 @@ package compose import ( + "bytes" + "crypto/sha256" + "encoding/hex" "encoding/json" "errors" "fmt" @@ -283,6 +286,28 @@ type ComposedHow struct { // HeldBefore is each module the node holds composing without the change (the candidate's base), held by the read or // blamed later for a refusal: the change refuses none of them (issue 774, review round 1). HeldBefore map[string]string + // Manifests is, by module taken in from its build, the ContentDigest of the manifest that build was made of (its + // build record's manifest), filled in the input a node answers a check with: the check's composer composes a module + // at its build only where the change's manifest is that one (issue 1921). Nil from a node-engine before it. + Manifests map[string]string +} + +// ContentDigest is the hex sha256 of a manifest's JSON in one form, its keys sorted and its spacing removed, its numbers +// as written: the same for the bytes its repository holds and for those bytes carried inside a request, which JSON +// encoding compacts on the way (a composition request through a build module), so the two are compared as one manifest. +func ContentDigest(raw []byte) (string, error) { + dec := json.NewDecoder(bytes.NewReader(raw)) + dec.UseNumber() + var v any + if err := dec.Decode(&v); err != nil { + return "", fmt.Errorf("compose: the manifest is not JSON: %w", err) + } + canonical, err := json.Marshal(v) + if err != nil { + return "", err + } + sum := sha256.Sum256(canonical) + return hex.EncodeToString(sum[:]), nil } // Place is where a node is, as its assignment says (inv.Overlay). diff --git a/internal/composer/composer.go b/internal/composer/composer.go index aeefe83..72e3db2 100644 --- a/internal/composer/composer.go +++ b/internal/composer/composer.go @@ -294,6 +294,53 @@ func overrule(base compose.Input, c Case, reads map[string]ManifestRead, shelf m awaited[name] = fmt.Sprintf("%s is %s (%s) and %s, and %s is awaited", name, held, why, judged, build) } } + // The same case where the running indexer cannot read the change's manifest at all (issue 1921, the group check of + // 8a80e8e86858): it gives that manifest no fingerprint, so the running node-engine composes the module at the build + // it runs, of the trunk's manifest, holds nothing and says nothing. Composing that build over the case's other + // manifests mixed two revisions (mesh-build-go's trunk build requiring a provision the change's manifests no longer + // name). A module the case carries that the input composes at a build of another manifest is composed from the + // case's manifest, its build awaited, named with the comparison that found it: the content digest of the manifest + // its build was made of where the input names one (ComposedHow.Manifests), else, from a node-engine before it, what + // the two manifests say once what a take-in resolves is left out (sameContract). + provided := map[string]bool{} + for _, name := range overlay.ProvidedModules() { + provided[name] = true // its manifest comes with mesh-sdk and always wins (runCase) + } + for name, built := range base.Catalogue { + if _, lifted := awaited[name]; lifted || provided[name] { + continue + } + if _, carried := c.Manifests[name]; !carried || !reads[name].Accepted || shelf[name].ReadNoFurther() { + continue + } + how := base.ComposedHow + var compared string + if digest := manifestOf(how, name); digest != "" { + if carried, err := compose.ContentDigest(c.Manifests[name].Raw); err == nil && carried == digest { + continue + } + compared = "its build was made of the manifest " + digest + ", not the change's" + } else { + if sameContract(built, shelf[name]) { + continue + } + compared = "its node-engine names no manifest its build was made of, and what the two manifests say differs " + + "beyond what a take-in resolves" + } + judged := "accepted by the check's own validator" + if _, given := c.Verdicts[name]; !given { + judged = "given no verdict by the check, so taken as accepted (novox/hq ADR 0578 §3)" + } + at := "" + if how != nil && how.Builds[name] != "" { + at = " " + how.Builds[name] + } + awaited[name] = "" + if compose.NamesBuild(shelf[name]) { + awaited[name] = fmt.Sprintf("%s is composed by the running node-engine at its build%s, of another manifest than the "+ + "change's (%s), and %s, and its build of the change's manifest is awaited", name, at, compared, judged) + } + } if len(awaited) == 0 { return base, awaited } @@ -329,12 +376,27 @@ func heldAgain(in, held compose.Input, awaited map[string]string) compose.Input if words == "" { continue } + why, was := held.Refused[name] + if !was { + // Composed by the running node-engine at a build of another manifest, not held (overrule): held here as a node + // holds a module whose build has not landed, the change's manifest as its stub, so that build is not laid over + // the change's other manifests again. + in.Catalogue[name] = compose.Stub(in.Catalogue[name]) + in.Refused[name] = words + if in.ComposedHow != nil { + if in.ComposedHow.Held == nil { + in.ComposedHow.Held = map[string]string{} + } + in.ComposedHow.Held[name] = words + } + continue + } if stub, ok := held.Catalogue[name]; ok { in.Catalogue[name] = stub } else { in.Catalogue[name] = compose.Stub(in.Catalogue[name]) } - in.Refused[name] = held.Refused[name] + in.Refused[name] = why if in.ComposedHow != nil && held.ComposedHow != nil { if why, was := held.ComposedHow.Held[name]; was { if in.ComposedHow.Held == nil { @@ -517,3 +579,28 @@ func SentInput(in compose.Input, node string) compose.Input { } return in } + +// manifestOf is the content digest of the manifest module's build was made of, as the input names it; "" when it names +// none (a node-engine before ComposedHow.Manifests, or a build it did not read). +func manifestOf(how *compose.ComposedHow, module string) string { + if how == nil { + return "" + } + return how.Manifests[module] +} + +// sameContract says whether the manifest a module was taken in from its build at (built) and the manifest a case carries +// for it (carried) compose alike: equal once what a take-in resolves (the build, its bundles, the templates filled into +// resources and items, what is handed) is left out of both. +func sameContract(built, carried catalogue.Manifest) bool { + contract := func(m catalogue.Manifest) string { + m.Handed, m.Build, m.Templates, m.Resources, m.Bundles, m.Items = nil, nil, "", nil, nil, nil + raw, err := json.Marshal(m) + if err != nil { + return "" + } + return string(raw) + } + b, c := contract(built), contract(carried) + return b != "" && b == c +} diff --git a/internal/composer/composer_test.go b/internal/composer/composer_test.go index ece1004..51bced2 100644 --- a/internal/composer/composer_test.go +++ b/internal/composer/composer_test.go @@ -301,6 +301,55 @@ func TestARequestItCannotRunIsRefused(t *testing.T) { } } }) + // Issue 1921 (the group check of 8a80e8e86858, ace: "nothing provides artifact-store, wanted by mesh-build-go"): a + // module the input composes at its build, taken in by a node-engine whose indexer cannot read the change's manifest, + // is composed at that build only where the build was made of the case's manifest: by the content digest the input + // names, else, from a node-engine that names none, by what the two manifests say beyond what a take-in resolves. + // Otherwise it is composed from the case's manifest, never silently: the node is not judged, naming it and how. + t.Run("a module composed at a build of another manifest", func(t *testing.T) { + plain := `{"module": "app", "version": "1", "resources": [{"id": "d", "type": "directory", "path": "/var/lib/app"}], ` + + `"build": {"artifacts": [{"name": "server", "kind": "upstream", "from": "nats:2.11@sha256:` + strings.Repeat("a", 64) + `"}]}}` + built, err := catalogue.Decode([]byte(`{"module": "app", "version": "1", "resources": [{"id": "built", "type": "directory", "path": "/var/lib/app-built"}]}`)) + if err != nil { + t.Fatal(err) + } + same, err := compose.ContentDigest([]byte(plain)) + if err != nil { + t.Fatal(err) + } + for _, row := range []struct { + name, raw, digest string + words string // in NotJudged, its build's paths not declared; "" composes at its build + }{ + {"the digest of the case's manifest", plain, same, ""}, + {"another digest, the same contract", plain, strings.Repeat("b", 64), "of another manifest than the change's (its build was made of the manifest " + strings.Repeat("b", 64)}, + {"no digest, the same contract", plain, "", ""}, + {"no digest, another contract", strings.Replace(plain, `"version": "1"`, `"version": "2"`, 1), "", + "app is composed by the running node-engine at its build fp1, of another manifest than the change's (its node-engine names no manifest"}, + {"a tool runner, its bundle awaited", `{"module": "app", "version": "1", "tool-runner": true, ` + + `"build": {"artifacts": [{"name": "runtime", "kind": "bundle", "language": "go", "from": "cmd/runner", "binary": "runner"}]}}`, "", + "composed with it held as its node-engine holds it, since it does not compose without its build: app is composed by the running node-engine at its build fp1"}, + } { + in := compose.Input{Catalogue: map[string]catalogue.Manifest{"app": built}, Nodes: []compose.Node{{Name: "a", Assigned: []string{"app"}}}, + ComposedHow: &compose.ComposedHow{Node: "a", Builds: map[string]string{"app": "fp1"}}} + if row.digest != "" { + in.ComposedHow.Manifests = map[string]string{"app": row.digest} + } + v, err := Run(withInput(t, Request{V: Version, Machines: []string{"a"}, Depth: DepthCompose, Declarations: true, + Cases: []Case{{Name: "change", Manifests: map[string]Manifest{"app": {Raw: json.RawMessage(row.raw)}}}}}, in)) + if err != nil { + t.Fatal(err) + } + m := v.Cases[0].Machines["a"] + declared := false + for _, p := range m.Declared { + declared = declared || p.Path == "/var/lib/app-built" + } + if row.words == "" && (!m.Composes || !declared) || row.words != "" && (m.Composes || declared || !strings.Contains(m.NotJudged, row.words)) { + t.Errorf("%s: machine %+v, want it naming %q", row.name, m, row.words) + } + } + }) v, err := Run(Request{V: Version, Cases: []Case{{Name: "only-read", Manifests: map[string]Manifest{"demo": {Raw: json.RawMessage(demo)}}}}}) if err != nil || len(v.Cases) != 1 || !v.Cases[0].Manifests["demo"].Accepted || v.Cases[0].Machines != nil || v.SDK == "" { t.Fatalf("a request with no machines: %+v, %v", v, err)