Keep a passing package's verdict once under its digest, so a second check can take it (issue 1287)

2026-10-21 21:02:54 +02:00
parent a5aa69a4e8
commit a00df90c47
8 changed files with 425 additions and 0 deletions
@@ -0,0 +1,22 @@
+module mesh-delivery-checker
+
+go 1.26.0
+
+require (
+	git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261021185947-afbe371003a5
+	github.com/nats-io/nats-server/v2 v2.11.17
+	github.com/nats-io/nats.go v1.54.0
+)
+
+require (
+	github.com/antithesishq/antithesis-sdk-go v0.7.0-default-no-op // indirect
+	github.com/google/go-tpm v0.9.8 // indirect
+	github.com/klauspost/compress v1.20.0 // indirect
+	github.com/minio/highwayhash v1.0.4 // indirect
+	github.com/nats-io/jwt/v2 v2.8.1 // indirect
+	github.com/nats-io/nkeys v0.4.16 // indirect
+	github.com/nats-io/nuid v1.0.1 // indirect
+	golang.org/x/crypto v0.57.0 // indirect
+	golang.org/x/sys v0.48.0 // indirect
+	golang.org/x/time v0.15.0 // indirect
+)
@@ -0,0 +1,29 @@
+git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261021100209-cb54756c3865 h1:VLir9NZbyi1LdH1YwWDIVRuL+lDC+AYlm6FZPlvmlm4=
+git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261021100209-cb54756c3865/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY=
+git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261021185947-afbe371003a5 h1:1BiPKRPTYpgex8rSHWT9az0BteOafGWRDzd/Ba6GAcA=
+git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261021185947-afbe371003a5/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY=
+github.com/antithesishq/antithesis-sdk-go v0.7.0-default-no-op h1:Z/MZK75wC/NSrkgqeNIa7jexam9uWzhLmFTSCPI/kn0=
+github.com/antithesishq/antithesis-sdk-go v0.7.0-default-no-op/go.mod h1:FQyySiasQQM8735Ddel3MRojmy4dA1IqCeyJ5jmPMbI=
+github.com/google/go-tpm v0.9.8 h1:slArAR9Ft+1ybZu0lBwpSmpwhRXaa85hWtMinMyRAWo=
+github.com/google/go-tpm v0.9.8/go.mod h1:h9jEsEECg7gtLis0upRBQU+GhYVH6jMjrFxI8u6bVUY=
+github.com/klauspost/compress v1.20.0 h1:a3C1ke2ohxFymNlb2HWAHjDeKCI90scRskErZkR0ezA=
+github.com/klauspost/compress v1.20.0/go.mod h1:LUdAzn7YLVvxLpc7y3V1m40wESHTgc1422pwwBSKYuI=
+github.com/minio/highwayhash v1.0.4 h1:asJizugGgchQod2ja9NJlGOWq4s7KsAWr5XUc9Clgl4=
+github.com/minio/highwayhash v1.0.4/go.mod h1:GGYsuwP/fPD6Y9hMiXuapVvlIUEhFhMTh0rxU3ik1LQ=
+github.com/nats-io/jwt/v2 v2.8.1 h1:V0xpGuD/N8Mi+fQNDynXohVvp7ZztevW5io8CUWlPmU=
+github.com/nats-io/jwt/v2 v2.8.1/go.mod h1:nWnOEEiVMiKHQpnAy4eXlizVEtSfzacZ1Q43LIRavZg=
+github.com/nats-io/nats-server/v2 v2.11.17 h1:GKEghcFK6A+aFx11Yf1LjgLC3txAwvyhnYzhBIQZA8I=
+github.com/nats-io/nats-server/v2 v2.11.17/go.mod h1:B1sFVz4StNosQ903ak4N1G01Fl/9f8e06mXpFIE2K24=
+github.com/nats-io/nats.go v1.54.0 h1:vsXoOxjHp/GmPUN+EcI7uOf/uB+iAP+kEsAFNQN0yzA=
+github.com/nats-io/nats.go v1.54.0/go.mod h1:y+DZoD1oBOYfZTU681eTUiUjI0vbqYGixNVFHcjHJ0k=
+github.com/nats-io/nkeys v0.4.16 h1:rd5oAuLOb8mnAycB0xleuEBNS1pVVnN0fv/FF34Eypg=
+github.com/nats-io/nkeys v0.4.16/go.mod h1:llLgWoI0o4z/Q57q2R1kHfmocyhGV6VG/U18Glg1Afs=
+github.com/nats-io/nuid v1.0.1 h1:5iA8DT8V7q8WK2EScv2padNa/rTESc1KdnPw4TC2paw=
+github.com/nats-io/nuid v1.0.1/go.mod h1:19wcPz3Ph3q0Jbyiqsd0kePYG7A95tJPxeL+1OSON2c=
+golang.org/x/crypto v0.57.0 h1:3ZVCjf8Ggz7zneR/EHRVx68Ctf+2pmIMP2UFhh9cC6M=
+golang.org/x/crypto v0.57.0/go.mod h1:Fdz0i5U6CoizGwLda9DttjSk6qlZo25zYNtR+ycvuZA=
+golang.org/x/sys v0.21.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
+golang.org/x/sys v0.48.0 h1:bbX/i/6MgT9BVLM9RT1thmxL04yeTAhbEz4SyadbXoo=
+golang.org/x/sys v0.48.0/go.mod h1:hNLxWAXmnKAxqDtdwIYC4bM9oQPEecfsnNMuSxOs3og=
+golang.org/x/time v0.15.0 h1:bbrp8t3bGUeFOx08pvsMYRTCVSMk89u4tKbNOZbp88U=
+golang.org/x/time v0.15.0/go.mod h1:Y4YMaQmXwGQZoFaVFk4YpCt4FLQMYKZe9oeV/f4MSno=
@@ -0,0 +1,6 @@
+// Package check is the checker's logic (novox/hq ADR 0484, ADR 0453 clause 4, ADR 0447, ADR 0444), apart from the bus: the
+// claim of a verdict key, the judging of a pull request's head merged onto its trunk through the build modules' fetch,
+// digest and test steps, the kept verdicts, the line of the status and the act the forge bridge performs. The bus is
+// reached through the small interfaces here, so each part is tested with fakes. The checker runs dark: nothing reads its
+// answers to merge or deliver (ADR 0453 clause 4).
+package check
@@ -0,0 +1,62 @@
+package check
+
+import (
+	"context"
+	"sort"
+	"sync"
+)
+
+// memKV is a bucket in memory with the revisions a real one gives.
+type memKV struct {
+	mu   sync.Mutex
+	vals map[string][]byte
+	revs map[string]uint64
+	next uint64
+	// creates counts the creates that succeeded, by key.
+	creates map[string]int
+}
+
+func newMemKV() *memKV {
+	return &memKV{vals: map[string][]byte{}, revs: map[string]uint64{}, creates: map[string]int{}}
+}
+
+func (m *memKV) Get(_ context.Context, key string) ([]byte, uint64, bool, error) {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	v, ok := m.vals[key]
+	return v, m.revs[key], ok, nil
+}
+
+func (m *memKV) Create(_ context.Context, key string, value []byte) (uint64, error) {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	if _, ok := m.vals[key]; ok {
+		return 0, ErrConflict
+	}
+	m.next++
+	m.vals[key], m.revs[key] = append([]byte(nil), value...), m.next
+	m.creates[key]++
+	return m.next, nil
+}
+
+func (m *memKV) Update(_ context.Context, key string, value []byte, revision uint64) (uint64, error) {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	if _, ok := m.vals[key]; !ok || m.revs[key] != revision {
+		return 0, ErrConflict
+	}
+	m.next++
+	m.vals[key], m.revs[key] = append([]byte(nil), value...), m.next
+	return m.next, nil
+}
+
+func (m *memKV) Keys(context.Context) ([]string, error) {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	keys := make([]string, 0, len(m.vals))
+	for k := range m.vals {
+		keys = append(keys, k)
+	}
+	sort.Strings(keys)
+	return keys, nil
+}
@@ -0,0 +1,90 @@
+package check
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"sort"
+	"time"
+
+	"git.novox.be/novox/mesh-sdk/go/delivery"
+)
+
+// Kept is the checker's bucket of kept verdicts (ADR 0444, ADR 0484 clause 13): a passing verdict per package and level,
+// under the digest the language's build module named for it. A key is written once, by a create that fails when it
+// exists, from the result the checker read from the test step and never from anything the code under test wrote; nothing
+// changes it afterwards.
+type Kept struct{ Store KV }
+
+// Lookup is the kept verdict of a level under a digest, or none.
+func (k Kept) Lookup(ctx context.Context, level, digest string) (delivery.Kept, bool, error) {
+	key, err := delivery.KeptKey(level, digest)
+	if err != nil {
+		return delivery.Kept{}, false, err
+	}
+	value, _, found, err := k.Store.Get(ctx, key)
+	if err != nil || !found {
+		return delivery.Kept{}, false, err
+	}
+	v, err := delivery.DecodeKept(value)
+	if err != nil {
+		return delivery.Kept{}, false, fmt.Errorf("%s holds what is not a kept verdict: %w", key, err)
+	}
+	return v, true, nil
+}
+
+// Keep writes v under level and digest by a create. It answers false, and writes nothing, when the key exists: the first
+// value stands.
+func (k Kept) Keep(ctx context.Context, level, digest string, v delivery.Kept) (bool, error) {
+	key, err := delivery.KeptKey(level, digest)
+	if err != nil {
+		return false, err
+	}
+	v.Level = level
+	data, err := delivery.EncodeKept(v)
+	if err != nil {
+		return false, err
+	}
+	if _, err := k.Store.Create(ctx, key, data); err != nil {
+		if errors.Is(err, ErrConflict) {
+			return false, nil
+		}
+		return false, fmt.Errorf("%s: %w", key, err)
+	}
+	return true, nil
+}
+
+// KeepPasses keeps, for the packages of a test result that passed with tests, a verdict under the digest the build module
+// named for the package at the level. A package that failed, has no tests or has no digest is never kept (ADR 0444 clause
+// 1): it runs every check. It answers the packages it kept now.
+func (k Kept) KeepPasses(ctx context.Context, repository, level, toolchain string, res Tested, digests map[string]string, at time.Time) ([]string, error) {
+	var kept []string
+	for _, p := range res.Packages {
+		digest, ok := digests[p.Package]
+		if !ok || !p.Passed || p.NoTests || p.Tests == 0 {
+			continue
+		}
+		if failedIn(res, p.Package) {
+			continue
+		}
+		made, err := k.Keep(ctx, level, digest, delivery.Kept{State: delivery.VerdictPass, Repository: repository, Package: p.Package,
+			Level: level, Tests: p.Tests, Took: int64(p.Elapsed * 1000), Toolchain: toolchain, At: at})
+		if err != nil {
+			return kept, err
+		}
+		if made {
+			kept = append(kept, p.Package)
+		}
+	}
+	sort.Strings(kept)
+	return kept, nil
+}
+
+func failedIn(res Tested, pkg string) bool {
+	for _, f := range res.Failed {
+		if f.Package == pkg {
+			return true
+		}
+	}
+	return false
+}
@@ -0,0 +1,102 @@
+package check
+
+import (
+	"context"
+	"strings"
+	"testing"
+	"time"
+
+	"git.novox.be/novox/mesh-sdk/go/delivery"
+)
+
+var at0 = time.Date(2026, 10, 21, 19, 0, 0, 0, time.UTC)
+
+const (
+	digA = "aa11aa11aa11aa11aa11aa11aa11aa11aa11aa11aa11aa11aa11aa11aa11aa11"
+	digB = "bb22bb22bb22bb22bb22bb22bb22bb22bb22bb22bb22bb22bb22bb22bb22bb22"
+)
+
+func keptValue(pkg string, tests int) delivery.Kept {
+	return delivery.Kept{State: delivery.VerdictPass, Repository: "novox/mesh-delivery", Package: pkg, Tests: tests, Took: 120, Toolchain: "go1.26.0", At: at0}
+}
+
+func TestAPassIsKeptByACreateUnderItsLevelAndDigest(t *testing.T) { // 1287/AC1
+	ctx, st := context.Background(), newMemKV()
+	k := Kept{Store: st}
+	made, err := k.Keep(ctx, delivery.LevelUnit, digA, keptValue("m/a", 3))
+	if err != nil || !made {
+		t.Fatalf("Keep = %v, %v, want made", made, err)
+	}
+	key, _ := delivery.KeptKey(delivery.LevelUnit, digA)
+	if st.creates[key] != 1 {
+		t.Fatalf("the key %s was created %d times, want once", key, st.creates[key])
+	}
+	got, found, err := k.Lookup(ctx, delivery.LevelUnit, digA)
+	if err != nil || !found || got.Package != "m/a" || got.Tests != 3 || got.Level != delivery.LevelUnit || got.Toolchain != "go1.26.0" {
+		t.Fatalf("Lookup = %+v, %v, %v", got, found, err)
+	}
+}
+
+func TestASecondKeepUnderTheSameDigestIsRefusedAndTheFirstStands(t *testing.T) { // 1287/AC2
+	ctx, st := context.Background(), newMemKV()
+	k := Kept{Store: st}
+	if made, err := k.Keep(ctx, delivery.LevelUnit, digA, keptValue("m/a", 3)); err != nil || !made {
+		t.Fatal(made, err)
+	}
+	made, err := k.Keep(ctx, delivery.LevelUnit, digA, keptValue("m/other", 9))
+	if err != nil || made {
+		t.Fatalf("the second Keep = %v, %v, want refused without an error", made, err)
+	}
+	got, _, _ := k.Lookup(ctx, delivery.LevelUnit, digA)
+	if got.Package != "m/a" || got.Tests != 3 {
+		t.Errorf("the first value did not stand: %+v", got)
+	}
+}
+
+func TestOnlyPassingPackagesWithADigestAreKept(t *testing.T) { // 1287/AC3, AC4
+	ctx, st := context.Background(), newMemKV()
+	k := Kept{Store: st}
+	res := Tested{Level: "unit", Passed: false, Packages: []GoPackage{
+		{Package: "m/a", Passed: true, Tests: 4, Elapsed: 0.2},
+		{Package: "m/b", Passed: false, Tests: 2},
+		{Package: "m/c", Passed: true, Tests: 1},      // no digest named
+		{Package: "m/d", Passed: true, NoTests: true}, // no tests
+	}, Failed: []Failure{{Package: "m/b", Test: "TestB"}}}
+	kept, err := k.KeepPasses(ctx, "novox/mesh-delivery", "unit", "go1.26.0", res, map[string]string{"m/a": digA, "m/b": digB, "m/d": digB}, at0)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if strings.Join(kept, ",") != "m/a" {
+		t.Fatalf("kept %v, want only m/a", kept)
+	}
+	if keys, _ := st.Keys(ctx); len(keys) != 1 {
+		t.Errorf("the bucket holds %v, want one key", keys)
+	}
+}
+
+func TestAKeptValueOfAnotherDigestIsNotRead(t *testing.T) { // 1287/AC7
+	ctx, st := context.Background(), newMemKV()
+	k := Kept{Store: st}
+	if _, err := k.Keep(ctx, delivery.LevelUnit, digA, keptValue("m/a", 3)); err != nil {
+		t.Fatal(err)
+	}
+	// A toolchain change changes the digest, so the new digest finds nothing and the old key is not deleted.
+	if _, found, _ := k.Lookup(ctx, delivery.LevelUnit, digB); found {
+		t.Error("a digest that was not kept was found")
+	}
+	if _, found, _ := k.Lookup(ctx, delivery.LevelIntegration, digA); found {
+		t.Error("a kept unit verdict was found at the integration level")
+	}
+	if keys, _ := st.Keys(ctx); len(keys) != 1 {
+		t.Errorf("the old key was removed: %v", keys)
+	}
+}
+
+func TestAKeptVerdictThatIsNotAPassIsRefused(t *testing.T) {
+	k := Kept{Store: newMemKV()}
+	v := keptValue("m/a", 1)
+	v.State = delivery.VerdictFail
+	if _, err := k.Keep(context.Background(), delivery.LevelUnit, digA, v); err == nil {
+		t.Error("a failure was kept")
+	}
+}
@@ -0,0 +1,95 @@
+package check
+
+import (
+	"encoding/json"
+	"strings"
+)
+
+// The shapes of what the build modules' steps answer, as their code writes them (mesh-catalog modules/mesh-build-go
+// cmd/mesh-build-go levels.go TestResult, digest.go DigestResult, fetch.go FetchResult; modules/mesh-build-node
+// cmd/mesh-build-node check.go TestResult, fetch.go FetchResult) with the fields the fetch step adds for a merge (ADR 0484
+// clause 3). Only the fields the checker reads are named; the rest is ignored.
+
+// MergeTree is what a fetch that merged says of one module directory: the tree sha, the digest of its manifest and the
+// hashes of its lock files by name. A bare string is the tree alone.
+type MergeTree struct {
+	Tree     string            `json:"tree"`
+	Manifest string            `json:"manifest,omitempty"`
+	Locks    map[string]string `json:"locks,omitempty"`
+}
+
+// UnmarshalJSON takes a string as the tree alone, or an object.
+func (t *MergeTree) UnmarshalJSON(data []byte) error {
+	if s := strings.TrimSpace(string(data)); strings.HasPrefix(s, `"`) {
+		return json.Unmarshal(data, &t.Tree)
+	}
+	type plain MergeTree
+	return json.Unmarshal(data, (*plain)(t))
+}
+
+// Fetched is the answer of a build module's fetch step.
+type Fetched struct {
+	Workspace string   `json:"workspace"`
+	Commit    string   `json:"commit"`
+	Modules   []string `json:"modules"`
+	// What a fetch with a base adds: the trunk commit, the merge commit, the files in conflict (a conflict is an answer),
+	// the paths the merge changes and the trees of the module directories.
+	Base      string               `json:"base,omitempty"`
+	Merge     string               `json:"merge,omitempty"`
+	Conflicts []string             `json:"conflicts,omitempty"`
+	Changed   []string             `json:"changed,omitempty"`
+	Trees     map[string]MergeTree `json:"trees,omitempty"`
+}
+
+// GoPackage is one package of a Go test result.
+type GoPackage struct {
+	Dir     string  `json:"dir"`
+	Package string  `json:"package"`
+	Passed  bool    `json:"passed"`
+	Elapsed float64 `json:"elapsed"`
+	Tests   int     `json:"tests"`
+	NoTests bool    `json:"no_tests,omitempty"`
+}
+
+// Failure is a test that failed, or a package that failed with no test to name.
+type Failure struct {
+	Package string `json:"package"`
+	Test    string `json:"test,omitempty"`
+	Output  string `json:"output,omitempty"`
+}
+
+// TestTime is one test and the seconds it took.
+type TestTime struct {
+	Package string  `json:"package"`
+	Test    string  `json:"test"`
+	Elapsed float64 `json:"elapsed"`
+}
+
+// Tested is the answer of a test step of either build module (the Go one names packages and counts tests; the Node one
+// names the package directory as Module and counts none).
+type Tested struct {
+	Level      string      `json:"level"`
+	Module     string      `json:"module,omitempty"`
+	Passed     bool        `json:"passed"`
+	Why        string      `json:"why,omitempty"`
+	NoTests    bool        `json:"no_tests,omitempty"`
+	Tests      int         `json:"tests"`
+	Skipped    int         `json:"skipped"`
+	Packages   []GoPackage `json:"packages,omitempty"`
+	Elapsed    float64     `json:"elapsed"`
+	Setup      float64     `json:"setup,omitempty"`
+	Bound      float64     `json:"bound"`
+	SetupBound float64     `json:"setup_bound,omitempty"`
+	Slowest    []TestTime  `json:"slowest,omitempty"`
+	Failed     []Failure   `json:"failed,omitempty"`
+	OverBound  bool        `json:"over_bound"`
+	OverWhich  string      `json:"over_which,omitempty"`
+}
+
+// Digested is the answer of the Go build module's digest step: one digest per package that has tests at the level, by import
+// path.
+type Digested struct {
+	Level        string            `json:"level"`
+	Packages     map[string]string `json:"packages"`
+	WithoutTests []string          `json:"without_tests,omitempty"`
+}
@@ -0,0 +1,19 @@
+package check
+
+import (
+	"context"
+	"errors"
+)
+
+// ErrConflict is what a KV answers when a Create finds the key or an Update finds another revision than the one given.
+var ErrConflict = errors.New("the key exists or changed")
+
+// KV is one of the checker's buckets: read, create and compare-and-set, each write answering the key's new revision. Only
+// the checker's credential writes them (ADR 0444 clause 5, ADR 0484 clause 2).
+type KV interface {
+	Get(ctx context.Context, key string) (value []byte, revision uint64, found bool, err error)
+	Create(ctx context.Context, key string, value []byte) (uint64, error)
+	Update(ctx context.Context, key string, value []byte, revision uint64) (uint64, error)
+	// Keys are every key of the bucket.
+	Keys(ctx context.Context) ([]string, error)
+}