Read the SDK fixtures at the pin of the tree under check, not the running controller's (issue 449 review)

A pull request moving the SDK passed its check against the old SDK and then failed
everywhere once it rolled out. In a merge check the test now reads the clone's history
at the tree's own go.mod pin, and holds the captured copy to the clone byte for byte.

cmd/mesh-controller/checks.go

@@ -276,8 +276,10 @@ func checkRequestFor(ctx context.Context, open *stores, p link.PullUpdated, scop
 // builds from; and beside the controller its main, for a judge the running controller predates, and the
 // lab's main, whose replays every check runs. **One rule, read by the check the controller asks for and by
 // the facts snapshot** (Facts.Beside), so a check run by hand clones what the build seat clones. Beside the
-// controller also the SDK, at the commit the controller's go.mod pins (sdkPinned): its conformance fixtures
-// are judged against the SDK the controller is built with, not one a desktop holds (novox/hq issue 449).
+// controller also the SDK, checked out at the commit the running controller's go.mod pins (sdkPinned), not
+// one a desktop holds (novox/hq issue 449). The checkout only places the clone: the conformance test reads the
+// fixtures at the pin of the tree under check, from the clone's history, so a pull request moving the SDK is
+// judged against the SDK it moves to (internal/link/conformance_test.go).
 func besideRefs(dir, running string) map[string]string {
 	switch dir {
 	case "mesh-catalog":

internal/link/conformance_sources_test.go

@@ -0,0 +1,128 @@
+package link
+
+import (
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// Where the conformance fixtures are read from, held on a small SDK repository of the test's own: two
+// commits, the clone checked out at the older as the build seat leaves it at the running controller's pin
+// (novox/hq issue 449). The contents are the test's, because what is judged is which commit is read.
+type sdkBed struct {
+	root, goMod, captured, capturedLog string
+	older, newer                       string
+}
+
+const fixtureName = "events/module-event.json"
+
+func newSDKBed(t *testing.T) sdkBed {
+	t.Helper()
+	if _, err := exec.LookPath("git"); err != nil {
+		t.Fatalf("git is needed to read the SDK at a pin, as a merge check does: %v", err)
+	}
+	b := sdkBed{root: t.TempDir()}
+	clone := filepath.Join(b.root, "mesh-sdk")
+	git := func(args ...string) string {
+		t.Helper()
+		cmd := exec.Command("git", append([]string{"-C", clone, "-c", "user.name=t", "-c", "user.email=t@t",
+			"-c", "commit.gpgsign=false"}, args...)...)
+		out, err := cmd.CombinedOutput()
+		if err != nil {
+			t.Fatalf("git %v: %v: %s", args, err, out)
+		}
+		return strings.TrimSpace(string(out))
+	}
+	write := func(dir, body string) {
+		t.Helper()
+		file := filepath.Join(dir, "conformance", filepath.FromSlash(fixtureName))
+		if err := os.MkdirAll(filepath.Dir(file), 0o755); err != nil {
+			t.Fatal(err)
+		}
+		if err := os.WriteFile(file, []byte(body), 0o644); err != nil {
+			t.Fatal(err)
+		}
+	}
+	if err := os.MkdirAll(clone, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	git("init", "--quiet")
+	write(clone, "older\n")
+	git("add", "-A")
+	git("commit", "--quiet", "-m", "older")
+	b.older = git("rev-parse", "HEAD")
+	write(clone, "newer\n")
+	git("commit", "--quiet", "-am", "newer")
+	b.newer = git("rev-parse", "HEAD")
+	git("checkout", "--quiet", b.older)
+
+	other := t.TempDir()
+	b.captured = filepath.Join(other, "mesh-sdk")
+	write(b.captured, "older\n")
+	b.capturedLog = filepath.Join(other, "CAPTURED")
+	if err := os.WriteFile(b.capturedLog, []byte("mesh-sdk  "+b.older+"  conformance/events\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	b.goMod = filepath.Join(other, "go.mod")
+	b.pin(t, b.newer)
+	return b
+}
+
+func (b sdkBed) pin(t *testing.T, commit string) {
+	t.Helper()
+	mod := "module x\n\nrequire (\n\t" + sdkModule + " v0.1.11-0.20261009143344-" + commit[:12] + "\n)\n"
+	if err := os.WriteFile(b.goMod, []byte(mod), 0o644); err != nil {
+		t.Fatal(err)
+	}
+}
+
+// A pull request moving the SDK is judged against the SDK it moves to: the clone is read at the tree's own
+// pin, not at what it is checked out at, the running controller's.
+func TestAMergeCheckReadsTheSDKAtTheTreesOwnPin(t *testing.T) {
+	b := newSDKBed(t)
+	raw, from, err := sdkFixture(b.root, b.goMod, b.captured, b.capturedLog, fixtureName)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if string(raw) != "newer\n" {
+		t.Errorf("read %q from %s; the tree pins the newer commit", raw, from)
+	}
+}
+
+// A captured copy that is not the SDK's at the commit CAPTURED names fails a merge check.
+func TestAMergeCheckFailsACapturedCopyEditedByHand(t *testing.T) {
+	b := newSDKBed(t)
+	if err := os.WriteFile(filepath.Join(b.captured, "conformance", filepath.FromSlash(fixtureName)),
+		[]byte("edited\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if _, _, err := sdkFixture(b.root, b.goMod, b.captured, b.capturedLog, fixtureName); err == nil ||
+		!strings.Contains(err.Error(), "is not mesh-sdk's at") {
+		t.Errorf("a hand-edited captured copy was accepted: %v", err)
+	}
+}
+
+// A pin the clone does not hold, or no clone at all, fails loudly and names what is missing.
+func TestAMergeCheckFailsWithoutTheSDKAtItsPin(t *testing.T) {
+	b := newSDKBed(t)
+	b.pin(t, "0123456789ab0123456789ab0123456789ab0123")
+	if _, _, err := sdkFixture(b.root, b.goMod, b.captured, b.capturedLog, fixtureName); err == nil ||
+		!strings.Contains(err.Error(), "0123456789ab") {
+		t.Errorf("a pin the clone lacks was not named: %v", err)
+	}
+	if _, _, err := sdkFixture(t.TempDir(), b.goMod, b.captured, b.capturedLog, fixtureName); err == nil ||
+		!strings.Contains(err.Error(), "not beside this check") {
+		t.Errorf("a missing clone was not refused: %v", err)
+	}
+}
+
+// Away from a merge check the captured copy is read, and nothing else.
+func TestAwayFromACheckTheCapturedCopyIsRead(t *testing.T) {
+	b := newSDKBed(t)
+	raw, _, err := sdkFixture("", b.goMod, b.captured, b.capturedLog, fixtureName)
+	if err != nil || string(raw) != "older\n" {
+		t.Errorf("read %q, %v; the captured copy holds the older", raw, err)
+	}
+}

internal/link/conformance_test.go

@@ -1,8 +1,12 @@
 package link
 
 import (
+	"bytes"
 	"encoding/json"
+	"fmt"
 	"os"
+	"os/exec"
+	"path"
 	"path/filepath"
 	"regexp"
 	"strings"
@@ -16,10 +20,19 @@ import (
 //
 // **Read from the SDK's own conformance directory, never copied by hand**: a fixture written into each
 // implementation is two fixtures, and two fixtures drift, which is the failure the suite exists to prevent.
-// Which SDK is read is named, never the one a desktop happens to hold beside this checkout (novox/hq issue
-// 449): in a merge check the SDK the build seat clones beside it, at the commit this controller's go.mod
-// pins, and a missing clone fails the test; elsewhere the copy captured in testdata/beside at that same
-// commit (internal/beside, testdata/beside/CAPTURED), held to go.mod by TestTheCapturedSDKIsTheOneGoModPins.
+// Which SDK is read is the one this tree's go.mod pins, never the one a desktop happens to hold beside this
+// checkout (novox/hq issue 449):
+//
+//   - in a merge check, the fixture as it stands at that pin in the mesh-sdk clone the build seat places
+//     beside the check — read with `git show`, because the clone is checked out at the *running*
+//     controller's pin, and a pull request moving the SDK must be judged against the SDK it moves to, or it
+//     passes the check and fails everywhere after it rolls out. A missing clone or a pin the clone lacks
+//     fails the test;
+//   - elsewhere, the copy captured in testdata/beside at the commit testdata/beside/CAPTURED names, held to
+//     go.mod by TestTheCapturedSDKIsTheOneGoModPins.
+//
+// In a merge check the captured copy is also compared with the clone at the captured commit, byte for byte,
+// so a hand-edited copy cannot pass for the SDK's.
 type fixture struct {
 	Name  string `json:"name"`
 	Given struct {
@@ -36,14 +49,24 @@ type fixture struct {
 	} `json:"wire"`
 }
 
+// sdkModule is the Go module of the SDK the controller is built against.
+const sdkModule = "git.novox.be/novox/mesh-sdk/go"
+
+// The tree's go.mod, and the captured copy, as this package finds them.
+var (
+	goModFile   = filepath.Join("..", "..", "go.mod")
+	capturedSDK = filepath.Join(beside.Captured(), "mesh-sdk")
+	capturedLog = filepath.Join(beside.Captured(), "CAPTURED")
+)
+
 func loadFixture(t *testing.T, name string) fixture {
 	t.Helper()
-	path := filepath.Join(beside.Dir(t, "mesh-sdk"), "conformance", name)
-	raw, err := os.ReadFile(path)
+	raw, from, err := sdkFixture(os.Getenv(beside.Env), goModFile, capturedSDK, capturedLog, name)
 	if err != nil {
 		// Failed, never skipped: a skip here passed the suite with nothing judged.
-		t.Fatalf("the SDK's conformance fixture %s: %v", name, err)
+		t.Fatal(err)
 	}
+	t.Logf("%s: %s", name, from)
 	var f fixture
 	if err := json.Unmarshal(raw, &f); err != nil {
 		t.Fatalf("%s: %v", name, err)
@@ -51,30 +74,119 @@ func loadFixture(t *testing.T, name string) fixture {
 	return f
 }
 
+// sdkFixture is one conformance fixture and where it was read: from the mesh-sdk clone in root (a merge
+// check's MESH_CHECK_BESIDE) at the pin of goMod, or, with no root, from the captured copy.
+func sdkFixture(root, goMod, captured, capturedLog, name string) ([]byte, string, error) {
+	file := path.Join("conformance", name)
+	if root == "" {
+		raw, err := os.ReadFile(filepath.Join(captured, filepath.FromSlash(file)))
+		if err != nil {
+			return nil, "", fmt.Errorf("the captured SDK's %s: %w", file, err)
+		}
+		return raw, "the copy captured in testdata/beside (see CAPTURED); a merge check reads the SDK's clone " +
+			"at this tree's pin", nil
+	}
+	clone := filepath.Join(root, "mesh-sdk")
+	if _, err := os.Stat(clone); err != nil {
+		return nil, "", fmt.Errorf("mesh-sdk is not beside this check in %s=%s, so the SDK this controller "+
+			"pins cannot be judged here: %w", beside.Env, root, err)
+	}
+	ref, err := sdkRef(goMod)
+	if err != nil {
+		return nil, "", err
+	}
+	raw, err := gitShow(clone, ref, file)
+	if err != nil {
+		return nil, "", fmt.Errorf("the mesh-sdk clone beside this check has no %s at %s, the SDK this tree's "+
+			"go.mod pins: %w", file, ref, err)
+	}
+	// The captured copy is the SDK's own, never edited by hand: the clone at the captured commit says so.
+	at, err := capturedCommit(capturedLog)
+	if err != nil {
+		return nil, "", err
+	}
+	theirs, err := gitShow(clone, at, file)
+	if err != nil {
+		return nil, "", fmt.Errorf("the mesh-sdk clone has no %s at %s, the commit CAPTURED names: %w", file, at, err)
+	}
+	ours, err := os.ReadFile(filepath.Join(captured, filepath.FromSlash(file)))
+	if err != nil {
+		return nil, "", fmt.Errorf("the captured SDK's %s: %w", file, err)
+	}
+	if !bytes.Equal(ours, theirs) {
+		return nil, "", fmt.Errorf("the captured %s is not mesh-sdk's at %s, the commit CAPTURED names: it was "+
+			"edited or captured wrong; capture it again as CAPTURED says", file, at)
+	}
+	return raw, "mesh-sdk cloned beside this check, at " + ref + " (this tree's go.mod)", nil
+}
+
+// gitShow is one file of a repository at a ref. safe.directory, because the clone is the build seat's and
+// the test may run as another user.
+func gitShow(repository, ref, file string) ([]byte, error) {
+	cmd := exec.Command("git", "-c", "safe.directory=*", "-C", repository, "show", ref+":"+file)
+	var stderr bytes.Buffer
+	cmd.Stderr = &stderr
+	out, err := cmd.Output()
+	if err != nil {
+		return nil, fmt.Errorf("git show %s:%s: %v: %s", ref, file, err, strings.TrimSpace(stderr.String()))
+	}
+	return out, nil
+}
+
+// pseudoCommit is the commit a Go pseudo-version names: v0.1.11-0.20261009143344-f047d0a4a970 → f047d0a4a970.
+var pseudoCommit = regexp.MustCompile(`-([0-9a-f]{12})$`)
+
+// sdkRef is the SDK repository's ref a go.mod pins: a pseudo-version's commit, or a release's tag (the SDK
+// tags its Go module under go/).
+func sdkRef(goMod string) (string, error) {
+	raw, err := os.ReadFile(goMod)
+	if err != nil {
+		return "", err
+	}
+	version := ""
+	for _, line := range strings.Split(string(raw), "\n") {
+		fields := strings.Fields(strings.TrimPrefix(strings.TrimSpace(line), "require "))
+		if len(fields) >= 2 && fields[0] == sdkModule {
+			version = fields[1]
+		}
+	}
+	if m := pseudoCommit.FindStringSubmatch(version); m != nil {
+		return m[1], nil
+	}
+	if strings.HasPrefix(version, "v") {
+		return "go/" + version, nil
+	}
+	return "", fmt.Errorf("%s pins no version of %s that names a commit or a tag", goMod, sdkModule)
+}
+
+// capturedCommit is the commit testdata/beside/CAPTURED names for mesh-sdk.
+func capturedCommit(capturedLog string) (string, error) {
+	raw, err := os.ReadFile(capturedLog)
+	if err != nil {
+		return "", err
+	}
+	at := regexp.MustCompile(`(?m)^mesh-sdk\s+([0-9a-f]{40})\s`).FindSubmatch(raw)
+	if at == nil {
+		return "", fmt.Errorf("%s names no commit for mesh-sdk", capturedLog)
+	}
+	return string(at[1]), nil
+}
+
 // The captured SDK is the one this controller is built against: when go.mod moves the SDK, the copy moves
 // with it, or the tests away from a merge check judge an SDK the controller no longer uses (novox/hq issue
 // 449).
 func TestTheCapturedSDKIsTheOneGoModPins(t *testing.T) {
-	mod, err := os.ReadFile(filepath.Join("..", "..", "go.mod"))
+	pinned, err := sdkRef(goModFile)
 	if err != nil {
 		t.Fatal(err)
 	}
-	pinned := regexp.MustCompile(`(?m)^\s*git\.novox\.be/novox/mesh-sdk/go v\S+-([0-9a-f]{12})$`).FindSubmatch(mod)
-	if pinned == nil {
-		t.Fatal("go.mod pins no commit of git.novox.be/novox/mesh-sdk/go as a pseudo-version; say here how a " +
-			"release's tag is matched against testdata/beside/CAPTURED")
-	}
-	captured, err := os.ReadFile(filepath.Join(beside.Captured(), "CAPTURED"))
+	at, err := capturedCommit(capturedLog)
 	if err != nil {
 		t.Fatal(err)
 	}
-	at := regexp.MustCompile(`(?m)^mesh-sdk\s+([0-9a-f]{40})\s`).FindSubmatch(captured)
-	if at == nil {
-		t.Fatal("testdata/beside/CAPTURED names no commit for mesh-sdk")
-	}
-	if !strings.HasPrefix(string(at[1]), string(pinned[1])) {
+	if strings.HasPrefix(pinned, "go/") || !strings.HasPrefix(at, pinned) {
 		t.Errorf("the SDK is captured at %s but go.mod pins %s: capture it again at the pinned commit, as "+
-			"testdata/beside/CAPTURED says", at[1], pinned[1])
+			"testdata/beside/CAPTURED says", at, pinned)
 	}
 }