From 96747105030d80ceff178f9cf0b53044ab5f1986 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 4 Nov 2026 16:22:59 +0100 Subject: [PATCH] F-delivery 1: the checker judges names against an answered empty packages state; judgeOver judges one digest An answered packages state with no name handed the judge nil, which judges no name, so a manifest naming an unknown package passed (DLV-I14, design 32). PackageNames is nil only until the state's first reading ends. judgeOver takes one module and digest, so dropping the known packages for a digest the trunk does not hold cannot drop them for another (J7's note). Claude-Session: https://claude.ai/code/session_01JG73KJNrH5EgFXMKKYSrig --- .../cmd/mesh-delivery-checker/judge_test.go | 8 +++++++ .../cmd/mesh-delivery-checker/main.go | 1 + .../cmd/mesh-delivery-checker/packages.go | 15 +++++++++++- .../internal/check/examiner_test.go | 8 +++++++ .../internal/check/judge.go | 3 ++- .../internal/check/manifests_judge.go | 9 ++++++-- .../internal/index/verdicts.go | 23 +++++++++++-------- 7 files changed, 54 insertions(+), 13 deletions(-) diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge_test.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge_test.go index 8d89a5a6..5b9d04cd 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge_test.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge_test.go @@ -124,6 +124,14 @@ func TestTheTypeScriptPackagesPathsComeFromTheIndexersModulesNotTheManifest(t *t // there and computed nowhere here: a key's module at its commit, a deleted key gone, an unreadable value left out. func TestThePublishedVersionsAreTheIndexersPackagesState(t *testing.T) { var p publishedPackages + // Not read yet, the state names no package and judges none; read and empty, it is an empty list judged against. + if got := p.PackageNames(); got != nil { + t.Errorf("before the first reading the known packages = %v, want nil", got) + } + p.Answered() + if got := p.PackageNames(); got == nil || len(got) != 0 { + t.Errorf("read and empty, the known packages = %#v, want an empty map", got) + } put := func(module, repository, dir, commit string) { key, _ := delivery.PackagesKey(module) raw, err := delivery.EncodePublishedPackages(delivery.PublishedPackages{Module: module, Repository: repository, Dir: dir, 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 059541db..628c9535 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go @@ -395,6 +395,7 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro if err != nil { return err } + published.Answered() // read whole, an empty state too: its names are judged from here on go func() { errs <- follow(ctx, pw, func(e jetstream.KeyValueEntry) { published.Put(e.Key(), e.Value(), !opOf(e)) }) }() diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/packages.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/packages.go index 9e8df54e..d56cb78c 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/packages.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/packages.go @@ -19,6 +19,15 @@ type publishedPackages struct { byKey map[string]delivery.PublishedPackages series map[string]delivery.SeriesPackage unread map[string]string + // answered is whether the state's first reading ended (Answered): until then the checker knows no package's name. + answered bool +} + +// Answered marks the state's first reading as ended, so PackageNames names what it holds, none included. +func (p *publishedPackages) Answered() { + p.mu.Lock() + defer p.mu.Unlock() + p.answered = true } // Put takes one key of the state in: its value, or its deletion. A value that does not decode is kept out and remembered @@ -68,10 +77,14 @@ func (p *publishedPackages) LatestPackages() delivery.PackageVersions { } // PackageNames are the packages the state's series keys name, by the name in its own ecosystem: the `/` -// each is built from (empty when the key does not say). Never nil, so the judge judges every name against it. +// each is built from (empty when the key does not say). Nil while the state is not read yet (Answered), which judges no +// name; once read never nil, an empty state included, so the judge judges every name against it. func (p *publishedPackages) PackageNames() map[string]string { p.mu.Lock() defer p.mu.Unlock() + if !p.answered { + return nil + } out := map[string]string{} for _, s := range p.series { if out[s.Name] == "" { diff --git a/modules/mesh-delivery-checker/internal/check/examiner_test.go b/modules/mesh-delivery-checker/internal/check/examiner_test.go index 45a27d46..081c050a 100644 --- a/modules/mesh-delivery-checker/internal/check/examiner_test.go +++ b/modules/mesh-delivery-checker/internal/check/examiner_test.go @@ -1046,6 +1046,9 @@ func TestTheIndexersJudgeJudgesTheChangesManifests(t *testing.T) { // known are the judge's known packages the request hands it (DLV-I14, design 32): the packages state's names and, // of web's package new in the group, the name its go.mod says at the member's head. known []string + // emptyState is a packages state answered and empty, no member declaring a package: the judge is handed an empty + // list, never nil, so every name a manifest's packages names is refused (review round 1, DLV-I14). + emptyState bool }{ {name: "a seat definition that does not parse", unparsed: "seats/web-api/seat.json: unexpected end of JSON input", fails: "novox/mesh-delivery#7: the seat definition of web-api (module web-contracts, go) does not parse: seats/web-api/seat.json: unexpected end of JSON input"}, @@ -1061,6 +1064,7 @@ func TestTheIndexersJudgeJudgesTheChangesManifests(t *testing.T) { {name: "a package series raised", series: "0.2", seriesWords: "web/pkg series 0.1 → 0.2"}, {name: "a package series kept", series: "0.1"}, {name: "a package new in the group", series: "0.1", known: []string{"git.novox.be/novox/mesh-sdk/go", "git.novox.be/novox/web/go"}}, + {name: "an answered packages state holding no name", emptyState: true, known: []string{}}, {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"}, @@ -1114,6 +1118,10 @@ func TestTheIndexersJudgeJudgesTheChangesManifests(t *testing.T) { Build: &catalogue.Build{Artifacts: []catalogue.Artifact{{Name: "pkg", Kind: catalogue.ArtifactPackage, Language: "go", Paths: []string{"go"}}}}}} b.ex.Forge = headFiles{repoA + " modules/web/go/go.mod " + head1: "module git.novox.be/novox/web/go\n\ngo 1.26\n"} } + if row.emptyState { + b.ex.KnownPackages = knownNames{} + b.ex.Read = fixedManifests{strings.Repeat("a", 64): {Module: "mail"}, strings.Repeat("b", 64): {Module: "web"}, web: {Module: "web"}} + } 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}} diff --git a/modules/mesh-delivery-checker/internal/check/judge.go b/modules/mesh-delivery-checker/internal/check/judge.go index ada57672..a7f0b8d4 100644 --- a/modules/mesh-delivery-checker/internal/check/judge.go +++ b/modules/mesh-delivery-checker/internal/check/judge.go @@ -101,7 +101,8 @@ type Examiner struct { } // KnownPackages names the packages the indexer's packages state knows (its series keys, delivery.SeriesPackage), by the // name in its own ecosystem, each with the `/` it is built from: the names a manifest's `packages` is - // judged against (manifests_judge.go, design 32). Nil hands the judge no names, and it judges none. + // judged against (manifests_judge.go, design 32). Nil, or PackageNames answering nil while the state is not read yet, + // hands the judge no names, and it judges none; an empty map is a state read and empty, against which every name fails. KnownPackages interface { PackageNames() map[string]string } diff --git a/modules/mesh-delivery-checker/internal/check/manifests_judge.go b/modules/mesh-delivery-checker/internal/check/manifests_judge.go index a2c88845..53c8cd98 100644 --- a/modules/mesh-delivery-checker/internal/check/manifests_judge.go +++ b/modules/mesh-delivery-checker/internal/check/manifests_judge.go @@ -266,17 +266,22 @@ func (st *state) judgedByMember(p Pull, merged []Fetched) { // packages state holds, and, of each package a member of the case declares that the state does not know by its // `/`, the name read at that member's head through the forge (packagename.Of, the indexer's own reader // at a merge). A name that cannot be read is said and left out, so a manifest naming it is refused naming it. Nil, which -// judges no name, when the checker follows no packages state. +// judges no name, when the checker follows no packages state or has not read it yet (PackageNames nil); an answered +// state that holds no name is an empty list, never nil, so every name is judged against it. func (st *state) knownPackages(ctx context.Context) []string { if st.j.KnownPackages == nil { return nil } names := st.j.KnownPackages.PackageNames() + if names == nil { + return nil + } built := map[string]bool{} for _, b := range names { built[b] = true } - known := slices.Collect(maps.Keys(names)) + known := make([]string, 0, len(names)) + known = append(known, slices.Collect(maps.Keys(names))...) declared, err := st.declaredPackages(ctx) if err != nil { st.c.note("the packages the group's members declare are not known, so only the packages state's names are known to the judge: " + err.Error()) diff --git a/modules/mesh-delivery-indexer/internal/index/verdicts.go b/modules/mesh-delivery-indexer/internal/index/verdicts.go index d60904f0..970a3e76 100644 --- a/modules/mesh-delivery-indexer/internal/index/verdicts.go +++ b/modules/mesh-delivery-indexer/internal/index/verdicts.go @@ -49,14 +49,19 @@ type Judged struct { // watchman raises as manifest-outgrown. A manifest in fresh (the digests a merge just took in) with no verdict yet, and // one already refused, is judged in full. func (ix *Indexer) JudgeAll(ctx context.Context, fresh map[string]bool) (Judged, error) { - return ix.judgeOver(ctx, fresh, nil) + return ix.judgeOver(ctx, fresh, "", "") } -// judgeOver is JudgeAll; with over (module to manifest digest) it judges those digests alone, each in the catalogue the -// modules state holds with its module's manifest replaced by it (a module the state does not hold added), and writes -// their verdicts and nothing else: no event, no count of the old sections, and no module's value but one that names the -// digest judged (carryVerdict). A digest of over with no verdict yet is judged in full, as a fresh one. -func (ix *Indexer) judgeOver(ctx context.Context, fresh map[string]bool, over map[string]string) (Judged, error) { +// judgeOver is JudgeAll; with a module and a manifest digest it judges that digest alone, in the catalogue the modules +// state holds with the module's manifest replaced by it (a module the state does not hold added), and writes its verdict +// and nothing else: no event, no count of the old sections, and no module's value but one that names the digest judged +// (carryVerdict). A digest with no verdict yet is judged in full, as a fresh one. One digest at a time, so the known +// packages dropped for a digest the trunk does not hold are dropped for that digest alone (review round 1, J7's note). +func (ix *Indexer) judgeOver(ctx context.Context, fresh map[string]bool, overModule, overDigest string) (Judged, error) { + var over map[string]string + if overModule != "" { + over = map[string]string{overModule: overDigest} + } // A merge, the start pass and indexer_judge may all judge at once: one pass at a time, so none writes a verdict from // modules another is rewriting. ix.judgeMu.Lock() @@ -73,7 +78,7 @@ func (ix *Indexer) judgeOver(ctx context.Context, fresh map[string]bool, over ma return out, err } trunkOf := map[string]string{} // a judged change's module to the trunk's manifest digest it changes (DLV-I15) - // The known packages a manifest's `packages` may name (DLV-I14). A digest of over the trunk does not hold (a pull + // The known packages a manifest's `packages` may name (DLV-I14). The digest judged the trunk does not hold (a pull // request's candidate, one the trunk no longer names) is not judged on them: the checker judges a change's names, // knowing its delivery group, whose new packages this state does not know yet (the lead's ruling, issue 1966). The // trunk's own manifests are, and a merge's are again in full (namesAtMerge). @@ -279,7 +284,7 @@ func (ix *Indexer) judgeDigests(ctx context.Context, over map[string]string, why if !ix.staleVerdict(ctx, over[module]) { continue } - if _, err := ix.judgeOver(ctx, nil, map[string]string{module: over[module]}); err != nil { + if _, err := ix.judgeOver(ctx, nil, module, over[module]); err != nil { ix.logf("judge: the manifest %s of %s (%s) is not judged: %v", over[module], module, why, err) } } @@ -583,7 +588,7 @@ func (ix *Indexer) JudgeAgain(ctx context.Context, module, digest string) TakenI return out } out.Module = kept.Module - if _, err := ix.judgeOver(ctx, nil, map[string]string{kept.Module: digest}); err != nil { + if _, err := ix.judgeOver(ctx, nil, kept.Module, digest); err != nil { out.Answer = "the manifest could not be judged: " + err.Error() return out }