chore: pin the corpus to a revision, and make a pin record its measurement

internal/corpus/corpus.go

@@ -331,9 +331,11 @@ func verifyAgainstPin(id string, doc []byte, pin Pin, source string) error {
 		return nil
 	}
 
-	// Parsed only to describe the mismatch. What a reviewer needs is what
-	// changed, not that a hash differed.
-	version, paths, operations := describe(doc)
+	// The mismatch is named by digest, which is what proves it. A reader
+	// reaching this has no parser to hand — Ensure is called from a test
+	// helper, not from a caller that decoded the document first — so the
+	// description degrades rather than the failure being withheld.
+	version, paths, operations := unparsed(doc)
 
 	return fmt.Errorf(`the pinned %s document is not what %s served.
   pinned  sha256:%s  version %s  %d path(s) / %d operation(s)
@@ -348,22 +350,25 @@ internal/corpus/testdata/corpus.lock.json`,
 		id)
 }
 
-// Describer parses a document enough to say what it is, for failure messages.
+// Describer parses a document enough to say what it is.
 //
-// A hook rather than a direct call on the OpenAPI parsing package, because
-// that package's own tests are in-package and are among this package's most
-// important consumers: importing it here would make them an import cycle. The
-// CLI installs the real parser; anything that has not is describing a failure
-// it is already reporting by digest.
-var Describer = func([]byte) (version string, paths, operations int) {
+// A parameter rather than a direct call on the OpenAPI parsing package,
+// because that package's own tests are in-package and are among this
+// package's most important consumers: importing it here would make them an
+// import cycle. specmodel.Describe is the implementation to pass.
+//
+// Every caller that writes a measurement down takes one, so a pin cannot
+// record what nothing measured.
+type Describer func(doc []byte) (version string, paths, operations int)
+
+// unparsed is the describer for a caller with no parser to hand: it reports
+// only what it can, which is nothing. Reserved for a failure message that
+// already names the change by digest, and never for a value written to the
+// lock.
+func unparsed([]byte) (version string, paths, operations int) {
 	return "unparsed", 0, 0
 }
 
-// describe reports what a document says about itself, best effort.
-func describe(doc []byte) (version string, paths, operations int) {
-	return Describer(doc)
-}
-
 func shortSHA(s string) string {
 	if len(s) <= 12 {
 		return s

internal/corpus/corpus_test.go

@@ -101,7 +101,35 @@ func TestUnit_Corpus_LockPinsTheDocumentsTheTestsRead(t *testing.T) {
 		if pin.PinnedAt.IsZero() {
 			t.Errorf("%s: no pinnedAt, so the cache directory name is not stable", id)
 		}
+		if pin.Version == "" || pin.Version == "unparsed" {
+			t.Errorf("%s: version %q, so the pin records no measurement of what it pinned", id, pin.Version)
+		}
+		if ref := movingRef(pin.UpstreamURL); ref != "" {
+			t.Errorf("%s: the upstream URL names %q, a ref that moves; pin a commit or a tag instead", id, ref)
+		}
+	}
+}
+
+// movingRefs are the branch names a source-hosting URL carries when it names
+// the tip of a branch rather than a fixed revision.
+var movingRefs = []string{"main", "master", "HEAD", "latest", "trunk", "develop"}
+
+// movingRef reports the moving ref an upstream URL names, empty when it names
+// none.
+//
+// A pin whose URL follows a branch re-pins itself every time the vendor
+// publishes: the hash stops matching, every test reading it fails, and the
+// only remedy is to restate the pin, which is the review the lock exists to
+// force. Pinning a revision makes the pin mean something.
+func movingRef(upstream string) string {
+	for _, segment := range strings.Split(upstream, "/") {
+		for _, ref := range movingRefs {
+			if segment == ref {
+				return ref
+			}
+		}
 	}
+	return ""
 }
 
 func slicesEqual(a, b []string) bool {
@@ -434,3 +462,30 @@ func TestUnit_Corpus_NoUserCacheDirFallsBackToTempNotToRelative(t *testing.T) {
 		t.Errorf("the fallback cache %q is not under tfpfgen/corpus", dir)
 	}
 }
+
+// TestUnit_Corpus_AMovingRefIsRejected proves the guard above catches the
+// shape it exists for, so it cannot pass by accident on a lock that happens
+// to hold no branch URL.
+func TestUnit_Corpus_AMovingRefIsRejected(t *testing.T) {
+	t.Parallel()
+
+	for _, url := range []string{
+		"https://raw.githubusercontent.com/o/r/main/spec.json",
+		"https://raw.githubusercontent.com/o/r/master/spec.json",
+		"https://example.invalid/latest/openapi.yaml",
+	} {
+		if movingRef(url) == "" {
+			t.Errorf("%s names a moving ref and was accepted", url)
+		}
+	}
+
+	for _, url := range []string{
+		"https://raw.githubusercontent.com/o/r/67c14c7efb01cdeeac0ecd8cee9fae8d7a80e2aa/spec.json",
+		"https://raw.githubusercontent.com/o/r/v2.1.0/spec.json",
+		"https://pubhub.devnetcloud.com/media/000-v7-apis/docs/reference/unified-oas/api.yaml",
+	} {
+		if ref := movingRef(url); ref != "" {
+			t.Errorf("%s names a fixed revision and was rejected as %q", url, ref)
+		}
+	}
+}

internal/corpus/refresh.go

@@ -57,18 +57,18 @@ func (u Upstream) Describe() string {
 // It never writes and never consults the cache: the question is what the
 // vendor is serving right now, which a cache would answer wrongly by
 // construction.
-func CheckUpstream(id string) (Upstream, error) {
+func CheckUpstream(id string, describe Describer) (Upstream, error) {
 	pin, err := PinFor(id)
 	if err != nil {
 		return Upstream{}, err
 	}
 
-	return checkPin(pin)
+	return checkPin(pin, describe)
 }
 
 // checkPin is CheckUpstream with the pin supplied, so RewriteLock can measure
 // against the on-disk lock rather than the embedded one.
-func checkPin(pin Pin) (Upstream, error) {
+func checkPin(pin Pin, describe Describer) (Upstream, error) {
 	doc, source, err := fetch(pin)
 	if err != nil {
 		return Upstream{}, err
@@ -100,7 +100,7 @@ func checkPin(pin Pin) (Upstream, error) {
 // Pins are measured against the on-disk lock rather than the embedded copy:
 // the embedded bytes are whatever was compiled in, and a rewrite must not
 // silently discard an edit made since.
-func RewriteLock(ids []string) error {
+func RewriteLock(ids []string, describe Describer) error {
 	path := LockPath()
 
 	raw, err := os.ReadFile(path) //nolint:gosec // a fixed name under the repository
@@ -134,7 +134,7 @@ func RewriteLock(ids []string) error {
 			return fmt.Errorf("%s has no pin for %q", path, id)
 		}
 
-		up, err := checkPin(pin)
+		up, err := checkPin(pin, describe)
 		if err != nil {
 			return fmt.Errorf("%s: %w", id, err)
 		}

internal/corpus/refresh_test.go

@@ -31,7 +31,7 @@ func TestUnit_Refresh_CheckUpstreamMeasuresWithoutJudging(t *testing.T) {
 
 	url, _ := serve(t, aDocument)
 
-	matching, err := checkPin(pinFor(t, url, aDocument))
+	matching, err := checkPin(pinFor(t, url, aDocument), unparsed)
 	if err != nil {
 		t.Fatalf("checkPin: %v", err)
 	}
@@ -40,7 +40,7 @@ func TestUnit_Refresh_CheckUpstreamMeasuresWithoutJudging(t *testing.T) {
 	}
 
 	moved := pinFor(t, url, "what the lock used to pin")
-	differing, err := checkPin(moved)
+	differing, err := checkPin(moved, unparsed)
 	if err != nil {
 		t.Fatalf("checkPin: %v", err)
 	}
@@ -60,12 +60,19 @@ func TestUnit_Refresh_CheckUpstreamMeasuresWithoutJudging(t *testing.T) {
 func TestUnit_Refresh_CheckUpstreamRefusesAnUnpinnedID(t *testing.T) {
 	t.Parallel()
 
-	if _, err := CheckUpstream("no-such-document"); err == nil ||
+	if _, err := CheckUpstream("no-such-document", unparsed); err == nil ||
 		!strings.Contains(err.Error(), "pins no document") {
 		t.Fatalf("CheckUpstream of an unpinned id: %v", err)
 	}
 }
 
+// stubDescriber stands where a real parser goes, answering values no fixture
+// could produce by accident so a pin carrying them proves the describer was
+// consulted.
+func stubDescriber([]byte) (version string, paths, operations int) {
+	return "9.9.9", 7, 11
+}
+
 // writeLock writes a lock file for RewriteLock tests and points EnvLockPath at
 // it.
 func writeLock(t *testing.T, content string) string {
@@ -118,7 +125,7 @@ func TestUnit_Refresh_RewriteLockRestatesOnlyThePinsThatMoved(t *testing.T) {
   }
 }`)
 
-	if err := RewriteLock([]string{"moved", "unmoved"}); err != nil {
+	if err := RewriteLock([]string{"moved", "unmoved"}, stubDescriber); err != nil {
 		t.Fatalf("RewriteLock: %v", err)
 	}
 
@@ -147,9 +154,15 @@ func TestUnit_Refresh_RewriteLockRestatesOnlyThePinsThatMoved(t *testing.T) {
 	if moved["unknownEntryKey"] != "kept too" {
 		t.Error("rewriting dropped an entry key it does not own")
 	}
-	// The default Describer cannot parse, and says so rather than guessing.
-	if moved["version"] != "unparsed" {
-		t.Errorf("the moved pin's version = %v", moved["version"])
+	// The pin records what the describer measured. A rewrite that ignored it
+	// would write the old version back, or none at all, and either reads as
+	// a document that did not move.
+	if moved["version"] != "9.9.9" {
+		t.Errorf("the moved pin's version = %v, want the describer's measurement", moved["version"])
+	}
+	if moved["pathCount"] != float64(7) || moved["operationCount"] != float64(11) {
+		t.Errorf("the moved pin's counts = %v/%v, want the describer's measurement",
+			moved["pathCount"], moved["operationCount"])
 	}
 
 	unmoved := entries["unmoved"].(map[string]any)
@@ -163,22 +176,22 @@ func TestUnit_Refresh_RewriteLockRestatesOnlyThePinsThatMoved(t *testing.T) {
 // and an unreachable upstream. None may write.
 func TestUnit_Refresh_RewriteLockFailsClosed(t *testing.T) {
 	t.Setenv(EnvLockPath, filepath.Join(t.TempDir(), "absent", LockFile))
-	if err := RewriteLock(nil); err == nil || !strings.Contains(err.Error(), "reading") {
+	if err := RewriteLock(nil, unparsed); err == nil || !strings.Contains(err.Error(), "reading") {
 		t.Errorf("a missing lock: %v", err)
 	}
 
 	writeLock(t, "{not json")
-	if err := RewriteLock(nil); err == nil || !strings.Contains(err.Error(), "parsing") {
+	if err := RewriteLock(nil, unparsed); err == nil || !strings.Contains(err.Error(), "parsing") {
 		t.Errorf("an unparsable lock: %v", err)
 	}
 
 	writeLock(t, `{"formatVersion": "1"}`)
-	if err := RewriteLock(nil); err == nil || !strings.Contains(err.Error(), "no openapi object") {
+	if err := RewriteLock(nil, unparsed); err == nil || !strings.Contains(err.Error(), "no openapi object") {
 		t.Errorf("a lock with no openapi object: %v", err)
 	}
 
 	writeLock(t, `{"openapi": {}}`)
-	if err := RewriteLock([]string{"ghost"}); err == nil || !strings.Contains(err.Error(), `no pin for "ghost"`) {
+	if err := RewriteLock([]string{"ghost"}, unparsed); err == nil || !strings.Contains(err.Error(), `no pin for "ghost"`) {
 		t.Errorf("an id the lock does not pin: %v", err)
 	}
 
@@ -194,7 +207,7 @@ func TestUnit_Refresh_RewriteLockFailsClosed(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if err := RewriteLock([]string{"dead"}); err == nil || !strings.Contains(err.Error(), "dead:") {
+	if err := RewriteLock([]string{"dead"}, unparsed); err == nil || !strings.Contains(err.Error(), "dead:") {
 		t.Errorf("an unreachable upstream: %v", err)
 	}
 	after, err := os.ReadFile(path) //nolint:gosec // a path this test built

internal/corpus/testdata/corpus.lock.json

@@ -7,9 +7,9 @@
       "mirrorUrl": "",
       "operationCount": 1220,
       "pathCount": 808,
-      "pinnedAt": "2026-08-07T14:30:00Z",
-      "sha256": "80850db290cde4eb487e0efb587cf27f305e77b6bef96933ed8a09b5169d5b1d",
-      "upstreamUrl": "https://raw.githubusercontent.com/github/rest-api-description/main/descriptions/api.github.com/api.github.com.json",
+      "pinnedAt": "2026-08-14T16:14:25.482999Z",
+      "sha256": "81c0ff2bbff9099b569058ef387e0ef83c33ef00ca0346e8a092777a71c192e3",
+      "upstreamUrl": "https://raw.githubusercontent.com/github/rest-api-description/67c14c7efb01cdeeac0ecd8cee9fae8d7a80e2aa/descriptions/api.github.com/api.github.com.json",
       "version": "1.1.4"
     },
     "thousandeyes": {

internal/specmodel/describe.go

@@ -0,0 +1,18 @@
+package specmodel
+
+// Describe reports what a document says about itself: the version its info
+// object declares, and how much surface it carries.
+//
+// It satisfies corpus.Describer, which is where a pin's recorded counts come
+// from. Those counts are what catch a truncated download that happens to
+// parse, so a document this cannot read reports zero rather than a guess.
+func Describe(doc []byte) (version string, paths, operations int) {
+	loaded, err := Load(doc)
+	if err != nil {
+		return "", 0, 0
+	}
+	for _, path := range loaded.Paths {
+		operations += len(path.Operations)
+	}
+	return loaded.Info.Version, len(loaded.Paths), operations
+}

internal/specmodel/describe_test.go

@@ -0,0 +1,34 @@
+package specmodel
+
+import "testing"
+
+// TestUnit_Specmodel_DescribeMeasuresWhatAPinRecords proves Describe reports
+// the three values a corpus pin is written from, and reports nothing rather
+// than a guess for bytes it cannot read — a truncated download must not pass
+// as a document with a plausible shape.
+func TestUnit_Specmodel_DescribeMeasuresWhatAPinRecords(t *testing.T) {
+	const doc = `openapi: 3.0.3
+info: {title: T, version: "4.5.6"}
+paths:
+  /widgets:
+    get:
+      responses:
+        "200": {description: ok}
+    post:
+      responses:
+        "201": {description: made}
+  /widgets/{id}:
+    delete:
+      responses:
+        "204": {description: gone}
+`
+	version, paths, operations := Describe([]byte(doc))
+	if version != "4.5.6" || paths != 2 || operations != 3 {
+		t.Errorf("Describe = %q, %d paths, %d operations; want 4.5.6, 2, 3", version, paths, operations)
+	}
+
+	version, paths, operations = Describe([]byte("\t{[ not a document"))
+	if version != "" || paths != 0 || operations != 0 {
+		t.Errorf("unreadable bytes described as %q, %d, %d", version, paths, operations)
+	}
+}