Keep a core repository's passing package under a digest that covers its siblings' commits, so core checks keep verdicts (issue 1287, C3a/C3b, lead decision L5)

The checker took and kept nothing for a workspace given siblings, and every core repository has siblings, so the kept store stayed empty. The digest step now folds the siblings' commits in and echoes them; a verdict is kept or taken only when the echo equals the siblings the fetch was given, so a digest that misses one never poisons another check (ADR 0444, issue 482).
2026-10-22 17:56:59 +02:00
parent d83156fb21
commit f7d8622deb
4 changed files with 92 additions and 11 deletions
@@ -19,6 +19,7 @@ const (
 	trunkA  = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
 	trunkA2 = "abababababababababababababababababababab"
 	trunkS  = "5555555555555555555555555555555555555555"
+	trunkS2 = "5656565656565656565656565656565656565656"
 )

 // candidateOf is a candidate of a group with the given pull requests, as the indexer writes one.
@@ -2,10 +2,13 @@ package check

 import (
 	"context"
+	"crypto/sha256"
+	"encoding/hex"
 	"encoding/json"
 	"fmt"
 	"os"
 	"path/filepath"
+	"sort"
 	"strings"
 	"sync"
 	"testing"
@@ -565,7 +568,7 @@ func trunksOf(t *testing.T, heads map[string]string) *Trunks {
 	return tr
 }

-func TestNothingIsKeptForAWorkspaceGivenSiblingsBeside(t *testing.T) { // review D1, ADR 0444, issue 1287
+func TestNothingIsKeptForAWorkspaceGivenSiblingsWhenTheDigestDoesNotSayItCoversThem(t *testing.T) { // review D1, ADR 0444, issue 1287
 	b := newBench(t)
 	b.ex.Trunks = trunksOf(t, map[string]string{repoSDK: trunkS, "novox/mesh-catalog": trunkA})
 	b.judge(t, caseOf(t, pullOf(repoA, 7, head1)))
@@ -573,7 +576,7 @@ func TestNothingIsKeptForAWorkspaceGivenSiblingsBeside(t *testing.T) { // review
 		t.Fatalf("the fetch was not given siblings: %+v", b.steps.tool("fetch"))
 	}
 	if keys, _ := b.kept.Keys(context.Background()); len(keys) != 0 {
-		t.Errorf("kept %v for a package whose tests may read a sibling the digest does not cover", keys)
+		t.Errorf("kept %v for a package whose digest does not say it covers the siblings", keys)
 	}
 	// And a kept verdict of the same digest from elsewhere is not taken for it: everything runs.
 	if _, err := (Kept{Store: b.kept}).Keep(context.Background(), "unit", digA, keptValue("fixture/slow", 2)); err != nil {
@@ -585,3 +588,64 @@ func TestNothingIsKeptForAWorkspaceGivenSiblingsBeside(t *testing.T) { // review
 		t.Errorf("%d test steps, want both levels run whole", n)
 	}
 }
+
+// besideDigest stands for the Go build module's digest over a workspace with siblings (DigestVersion 3, lead decision L5):
+// each package's digest folds the siblings' commits in, and the answer echoes the lines it folded.
+func besideDigest(b *bench) func(c stepCall) (any, error) {
+	return func(c stepCall) (any, error) {
+		level := c.args["level"].(string)
+		var lines []string
+		for _, f := range b.steps.tool("fetch") {
+			refs, _ := f.args["beside"].([]BesideRef)
+			for _, r := range refs {
+				lines = append(lines, r.Repository+" "+r.Commit)
+			}
+		}
+		sort.Strings(lines)
+		sum := sha256.Sum256([]byte(level + "\n" + strings.Join(lines, "\n")))
+		return Digested{Level: level, Packages: map[string]string{"fixture/slow": hex.EncodeToString(sum[:])}, Beside: lines}, nil
+	}
+}
+
+func TestACoreRepositorysPassingPackageIsKeptUnderADigestCoveringItsSiblings(t *testing.T) { // 1287 AC1-AC3, AC7 with L5
+	b := newBench(t)
+	b.steps.handlers["build_go_digest"] = besideDigest(b)
+	b.ex.Trunks = trunksOf(t, map[string]string{repoSDK: trunkS, "novox/mesh-catalog": trunkA})
+	b.judge(t, caseOf(t, pullOf(repoA, 7, head1)))
+	keys, _ := b.kept.Keys(context.Background())
+	if len(keys) != 2 {
+		t.Fatalf("kept %v, want the passing package at both levels", keys)
+	}
+	// The same siblings again: everything is taken, nothing runs.
+	b.steps.calls = nil
+	v := b.judge(t, caseOf(t, pullOf(repoA, 7, head1))).Verdict
+	if n := len(b.steps.tool("test")); n != 0 || v.Levels[delivery.LevelUnit].Run != 0 || v.Levels[delivery.LevelUnit].Kept == 0 {
+		t.Fatalf("%d test steps, unit %+v, want none run and the kept verdict taken", n, v.Levels[delivery.LevelUnit])
+	}
+	// A sibling's commit moves: a new digest, so the package runs again and is kept under it.
+	b.ex.Trunks = trunksOf(t, map[string]string{repoSDK: trunkS2, "novox/mesh-catalog": trunkA})
+	b.steps.calls = nil
+	b.judge(t, caseOf(t, pullOf(repoA, 7, head1)))
+	if n := len(b.steps.tool("test")); n != 2 {
+		t.Errorf("%d test steps after a sibling moved, want both levels run", n)
+	}
+	if keys, _ := b.kept.Keys(context.Background()); len(keys) != 4 {
+		t.Errorf("kept %v, want a second key per level under the new digest", keys)
+	}
+}
+
+func TestADigestThatEchoesOtherSiblingsKeepsNothing(t *testing.T) { // 1287 with L5: an echo that differs is not trusted
+	b := newBench(t)
+	inner := besideDigest(b)
+	b.steps.handlers["build_go_digest"] = func(c stepCall) (any, error) {
+		v, err := inner(c)
+		d := v.(Digested)
+		d.Beside = d.Beside[:len(d.Beside)-1]
+		return d, err
+	}
+	b.ex.Trunks = trunksOf(t, map[string]string{repoSDK: trunkS, "novox/mesh-catalog": trunkA})
+	b.judge(t, caseOf(t, pullOf(repoA, 7, head1)))
+	if keys, _ := b.kept.Keys(context.Background()); len(keys) != 0 {
+		t.Errorf("kept %v under a digest that does not cover every sibling", keys)
+	}
+}
@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"path"
+	"slices"
 	"sort"
 	"strings"
 	"time"
@@ -87,6 +88,16 @@ func (j *Examiner) logf(format string, args ...any) {
 	}
 }

+// besideLines are the siblings as the digest step echoes them: `<owner/repo> <commit>`, sorted.
+func besideLines(refs []BesideRef) []string {
+	out := make([]string, 0, len(refs))
+	for _, r := range refs {
+		out = append(out, r.Repository+" "+r.Commit)
+	}
+	sort.Strings(out)
+	return out
+}
+
 // target is a pull request on one build module: the node chosen, the workspace its fetch made and what it said.
 type target struct {
 	pull      Pull
@@ -98,8 +109,9 @@ type target struct {
 	fetchArgs map[string]any
 	// priority are the Go packages (import paths) or the Node module directories whose test files the merge changes.
 	changed []string
-	// beside says the workspace was given siblings: its tests may read what the digest does not cover, so nothing is kept for it.
-	beside bool
+	// beside are the lines `<owner/repo> <commit>` of the siblings the workspace was given, sorted: its tests may read them,
+	// so a verdict is kept or taken only under a digest that says it folded exactly these in (lead decision L5).
+	beside []string
 	plans  map[string]*goPlan // by level
 }

@@ -272,7 +284,7 @@ func (st *state) fetchAll(ctx context.Context) *delivery.Verdict {
 				}
 				if len(beside) > 0 {
 					args["beside"] = beside
-					t.beside = true
+					t.beside = besideLines(beside)
 				}
 			}
 			raw, err := st.step(ctx, t, "fetch", fmt.Sprintf("fetch %s#%d on %s", p.Repository, p.Number, node), args)
@@ -398,12 +410,6 @@ func (st *state) planGo(ctx context.Context, t *target, level string) *delivery.
 	}
 	plan := &goPlan{digests: map[string]string{}}
 	t.plans[level] = plan
-	if t.beside {
-		// The digest covers the package's own files, the toolchain and the header, not the siblings placed beside the tree: a
-		// package whose tests read one must never be kept, nor taken from a verdict kept for other inputs (ADR 0444).
-		plan.all = true
-		return nil
-	}
 	raw, err := st.step(ctx, t, "digest", fmt.Sprintf("digest the %s level of %s#%d on %s", level, t.pull.Repository, t.pull.Number, t.node),
 		map[string]any{"workspace": t.fetched.Workspace, "level": level})
 	var d Digested
@@ -421,6 +427,13 @@ func (st *state) planGo(ctx context.Context, t *target, level string) *delivery.
 		plan.all = true
 		return nil
 	}
+	if !slices.Equal(d.Beside, t.beside) {
+		// The siblings the tests read are not all in the digest (a build module before DigestVersion 3, or another set): a
+		// package whose tests read one is never kept, nor taken from a verdict kept for other inputs (ADR 0444, L5).
+		st.j.logf("%s#%d: the %s level's digest covers the siblings %v, not %v, so it runs whole and keeps nothing", t.pull.Repository, t.pull.Number, level, d.Beside, t.beside)
+		plan.all = true
+		return nil
+	}
 	plan.digests = d.Packages
 	a := st.levels[level]
 	pkgs := make([]string, 0, len(d.Packages))
@@ -89,6 +89,9 @@ type Tested struct {
 // 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 {
+	// Beside are the lines `<owner/repo> <commit>` of the siblings the digest folded in (DigestVersion 3, lead decision L5),
+	// sorted; absent when the workspace has none or the build module folds none in.
+	Beside       []string          `json:"beside,omitempty"`
 	Level        string            `json:"level"`
 	Packages     map[string]string `json:"packages"`
 	WithoutTests []string          `json:"without_tests,omitempty"`