From c47f0af0ab26cb6123b1bcb12df5fa4a101523c8 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 4 Nov 2026 15:44:36 +0100 Subject: [PATCH] J7 (3): the indexer's verdict on a pull request's candidate does not refuse on a package's name The checker, which knows the delivery group, judges a change's names; the indexer's state knows only the trunk's packages and refused a package new in the group. The trunk's manifests are still judged on them, and a merge's manifest in full, though its candidate was accepted without them (UnknownPackageNames). Claude-Session: https://claude.ai/code/session_01JG73KJNrH5EgFXMKKYSrig --- .../internal/index/verdicts.go | 25 +++++++++++++-- .../internal/index/verdicts_test.go | 31 +++++++++++++++++++ .../mesh-delivery-indexer/judge/packages.go | 25 ++++++++++++--- 3 files changed, 73 insertions(+), 8 deletions(-) diff --git a/modules/mesh-delivery-indexer/internal/index/verdicts.go b/modules/mesh-delivery-indexer/internal/index/verdicts.go index 88d9a58a..d60904f0 100644 --- a/modules/mesh-delivery-indexer/internal/index/verdicts.go +++ b/modules/mesh-delivery-indexer/internal/index/verdicts.go @@ -73,6 +73,11 @@ 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 + // 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). + known := ix.knownPackages(ctx) if over != nil { fresh = map[string]bool{} for module, digest := range over { @@ -80,6 +85,9 @@ func (ix *Indexer) judgeOver(ctx context.Context, fresh map[string]bool, over ma if m.Manifest != "" && m.Manifest != digest { trunkOf[module] = m.Manifest } + if m.Manifest != digest { + known = nil + } m.Manifest = digest held[module] = m fresh[digest] = true @@ -114,7 +122,7 @@ func (ix *Indexer) judgeOver(ctx context.Context, fresh map[string]bool, over ma if ix.OldSections != nil { counts = ix.heldOldSections(ctx) } - refused, waits, unnamed := judge.ShelfWaiting(raws, predefined, 0, counts, ix.knownPackages(ctx)) + refused, waits, unnamed := judge.ShelfWaiting(raws, predefined, 0, counts, known) // A change's renamed-from is judged against the trunk's manifest of the same module (DLV-I15, design 32). for module, digest := range trunkOf { if _, waiting := waits[module]; waiting || raws[module] == nil { @@ -166,7 +174,7 @@ func (ix *Indexer) judgeOver(ctx context.Context, fresh map[string]bool, over ma } } } else if problems := refused[name]; len(problems) > 0 { - if ix.acceptedBefore(ctx, digests[name], fresh) { + if ix.acceptedBefore(ctx, digests[name], fresh) && !namesAtMerge(name, raws[name], fresh[digests[name]] && over == nil, known) { v.Warnings = problems out.Outgrown = append(out.Outgrown, name) } else { @@ -305,7 +313,17 @@ func (ix *Indexer) judgeStale(ctx context.Context) { } } -// kickJudge judges again a start pass that failed, when the indexer hears something: never on a timer. +// namesAtMerge says whether a manifest a merge takes in (atMerge) names a package known does not: it is refused for it even +// when its digest was accepted before, since that verdict was its candidate's, judged without the names (judgeOver), so +// no stricter judge turns an acceptance into a refusal here (ADR 0579 ยง1). +func namesAtMerge(module string, raw []byte, atMerge bool, known judge.KnownPackages) bool { + if !atMerge || known == nil { + return false + } + m, err := catalogue.Decode(raw) + return err == nil && len(judge.UnknownPackageNames(module, m, known)) > 0 +} + // knownPackages are the packages a manifest's `packages` may name (DLV-I14, design 32): every name the packages state's // series keys hold, as KeepPackageNames reads it at each merge. Nil, which judges no name, where the indexer keeps no // packages state or it cannot be listed (said): a state the judge cannot read refuses no manifest. @@ -335,6 +353,7 @@ func (ix *Indexer) knownPackages(ctx context.Context) judge.KnownPackages { return known } +// kickJudge judges again a start pass that failed, when the indexer hears something: never on a timer. func (ix *Indexer) kickJudge(ctx context.Context) { if !ix.judgePending.Load() { return diff --git a/modules/mesh-delivery-indexer/internal/index/verdicts_test.go b/modules/mesh-delivery-indexer/internal/index/verdicts_test.go index ddc4a105..756c92ee 100644 --- a/modules/mesh-delivery-indexer/internal/index/verdicts_test.go +++ b/modules/mesh-delivery-indexer/internal/index/verdicts_test.go @@ -240,6 +240,37 @@ func TestTheIndexerJudgesEveryManifestOnce(t *testing.T) { t.Errorf("the change %s: %+v, want refused naming %q", raw, v, want) } } + // DLV-I14 and the lead's ruling (issue 1966): a pull request's candidate is not refused for naming a package the + // packages state does not know (the checker judges its names, knowing its delivery group); the same manifest a merge + // takes in is, though its candidate was accepted; a name the state knows is accepted at both. + ix.Packages = newMem() + sdkGo := "git.novox.be/novox/mesh-sdk/go" + if value, err := delivery.EncodeSeriesPackage(delivery.SeriesPackage{V: 1, Name: sdkGo, Package: "mesh-sdk/go", Series: "0.1"}); err != nil { + t.Fatal(err) + } else { + ix.Packages.(*memStore).put(SeriesKey(sdkGo, "0.1"), value) + } + for raw, refusedAtMerge := range map[string]bool{ + `{"module":"pkguser","version":"1","packages":{"@novox/ui":"0.1"}}`: true, + `{"module":"pkguser","version":"2","packages":{"` + sdkGo + `":"0.1"}}`: false, + } { + head, err := ix.keepManifest(ctx, "pkguser", raw) + if err != nil { + t.Fatal(err) + } + ix.judgeDigests(ctx, map[string]string{"pkguser": head}, "novox/mesh-catalog#1") + if v := verdict(head); v.Refused() { + t.Errorf("the candidate %s is refused: %+v", raw, v) + } + merged := hold("pkguser", raw) + if _, err := ix.JudgeAll(ctx, map[string]bool{merged: true}); err != nil { + t.Fatal(err) + } + if v := verdict(merged); v.Refused() != refusedAtMerge || refusedAtMerge && !strings.Contains(strings.Join(v.Problems, " "), `name "@novox/ui", which no package`) { + t.Errorf("the merge of %s: %+v, want refused %v for the name", raw, v, refusedAtMerge) + } + } + ix.Packages = nil // 1966/AC11: modules using state and none using logs but the foundation: logs counts 0, so a manifest of another // module using logs is refused and the foundation's is accepted. ix.OldSections = newMem() diff --git a/modules/mesh-delivery-indexer/judge/packages.go b/modules/mesh-delivery-indexer/judge/packages.go index cfacae8c..03cce2b9 100644 --- a/modules/mesh-delivery-indexer/judge/packages.go +++ b/modules/mesh-delivery-indexer/judge/packages.go @@ -36,12 +36,8 @@ func PackagesCatalogueProblems(shelf catalogue.Shelf, known KnownPackages) []str var problems []string for _, module := range shelfOrder(shelf) { accepts := shelf[module].Packages + problems = append(problems, UnknownPackageNames(module, shelf[module], known)...) for _, name := range slices.Sorted(maps.Keys(accepts)) { - if known != nil && !known[name] { - problems = append(problems, fmt.Sprintf("%s's packages name %q, which no package of the mesh is named "+ - "(a package is named in its own language: a Go package by its go.mod's module path, a TypeScript "+ - "package by its package.json's name; design 32)", module, name)) - } if s := strings.TrimSpace(accepts[name]); !seriesForm.MatchString(s) { problems = append(problems, fmt.Sprintf("%s's packages accept %q in the series %q, which is no . "+ "such as 0.1 (design 32)", module, name, accepts[name])) @@ -50,3 +46,22 @@ func PackagesCatalogueProblems(shelf catalogue.Shelf, known KnownPackages) []str } return problems } + +// UnknownPackageNames are the problems of module's manifest m whose `packages` name a package known does not (DLV-I14, +// design 32), one per name, sorted; none when known is nil. PackagesCatalogueProblems says them among the rest; the indexer +// asks them of a manifest a merge takes in, whose candidate was judged without them (the checker judges a pull request's +// names, knowing its delivery group). +func UnknownPackageNames(module string, m catalogue.Manifest, known KnownPackages) []string { + if known == nil { + return nil + } + var problems []string + for _, name := range slices.Sorted(maps.Keys(m.Packages)) { + if !known[name] { + problems = append(problems, fmt.Sprintf("%s's packages name %q, which no package of the mesh is named "+ + "(a package is named in its own language: a Go package by its go.mod's module path, a TypeScript "+ + "package by its package.json's name; design 32)", module, name)) + } + } + return problems +}