indexer_take_in reads a held module again only when its record is behind the source's head, and the two new tests are rows (issue 1126, review)

A head older than the record, or one whose order the forge cannot say, would have written
an older manifest back. The new tests become rows of existing ones: the repository is over
its 500 unit tests.

modules/mesh-delivery-forge-bridge/internal/bridge/files_test.go

@@ -120,6 +120,8 @@ func TestMergedCarriesManifestsAndLocks(t *testing.T) { // 1448/AC1
 	if _, err := ConvertWith(mergedSubject, catalogue, treesOf(&calls, "r0", map[string]string{"modules/a": "t1"}, nil), forge.get, failing); !errors.Is(err, ErrRetry) {
 		t.Errorf("a listing the forge cannot give: %v", err)
 	}
+	// Issue 1126: a merge reads the forge only for the module directories whose tree changed.
+	t.Run("a merge reads only the directories whose tree changed", mergeReadsOnlyChangedDirs)
 }
 
 func TestPullWithKnownDirsCarriesTheSameAtTheHead(t *testing.T) { // 1448/AC2
@@ -378,7 +380,7 @@ func (k memKept) Put(tree string, in Inputs)     { k[tree] = in }
 
 // Issue 1126 (mesh-catalog#535; issue 1458 met the kind first): a merge reads the forge only for the module directories
 // whose tree changed. Its message is the same as one read whole; a directory whose tree is kept costs no read.
-func TestAMergeReadsOnlyTheDirectoriesWhoseTreeChanged(t *testing.T) {
+func mergeReadsOnlyChangedDirs(t *testing.T) {
 	manifest, sum := real(t, "module.json"), real(t, "go.sum")
 	forge := &forgeFiles{have: map[string][]byte{"modules/a/module.json": manifest, "modules/a/go.sum": sum, "modules/b/module.json": []byte(`{"module":"b"}`)}}
 	kept := memKept{}

modules/mesh-delivery-indexer/internal/index/registered.go

@@ -313,6 +313,22 @@ func (ix *Indexer) takeInHeld(ctx context.Context, module string) (TakenIn, bool
 	if head == "" || head == cur.Commit {
 		return out, false
 	}
+	// Only forward: a record at a merge the source does not name yet (a merge applied before the source moved) is never
+	// written back to the source's older head. Where the order cannot be known, nothing is read.
+	behind, why := false, "the forge cannot be asked"
+	if ix.Contains != nil {
+		var err error
+		if behind, err = ix.Contains(ctx, cur.Repository, cur.Commit, head); err != nil {
+			why = err.Error()
+		} else if !behind {
+			why = fmt.Sprintf("its record at %.12s is not in %.12s", cur.Commit, head)
+		}
+	}
+	if !behind {
+		out.Held, out.Repository, out.Commit, out.Path = true, cur.Repository, cur.Commit, cur.Path
+		out.Answer = fmt.Sprintf("held; the source names another head, %.12s, and not one its record is behind (%s), so it is not read again", head, why)
+		return out, true
+	}
 	t, err := ix.indexAt(ctx, Registration{Module: module, Repository: cur.Repository, Path: cur.Path, Commit: head}, true, true)
 	out.Held, out.Repository, out.Commit, out.Path = true, t.repository, t.commit, pathOf(t.dir)
 	switch {

modules/mesh-delivery-indexer/internal/index/registered_test.go

@@ -111,6 +111,58 @@ func TestARegisteredModuleIsIndexedFromItsRepository(t *testing.T) { // the gran
 			}
 		})
 	}
+
+	// Issue 1126: mesh-catalog#535 changed mesh-vault, but its merged message never reached the indexer, so the record
+	// stood at the merge before. A take-in of the held module reads it again at the source's head, only when the record is
+	// behind that head: a head older than the record (a merge applied before the source moved), or one whose order the forge
+	// cannot say, is never written back.
+	for _, tc := range []struct {
+		name     string
+		contains func(context.Context, string, string, string) (bool, error)
+		moved    bool
+	}{
+		{name: "a held module behind the source's head is read again", moved: true,
+			contains: func(_ context.Context, _, commit, in string) (bool, error) { return commit == merge && in == h2, nil }},
+		{name: "a source head older than the record is not read", contains: func(context.Context, string, string, string) (bool, error) { return false, nil }},
+		{name: "a forge that cannot say the order reads nothing", contains: func(context.Context, string, string, string) (bool, error) {
+			return false, errors.New("timeout")
+		}},
+		{name: "no forge to ask reads nothing"},
+	} {
+		t.Run("take-in: "+tc.name, func(t *testing.T) {
+			ctx := context.Background()
+			ix, mods := modulesIndexer(t)
+			ix.Manifests = newMem()
+			if err := ix.Apply(ctx, mergedMsg("novox/mesh-catalog", map[string]string{"modules/a": "treeA"})); err != nil {
+				t.Fatal(err)
+			}
+			ix.source = sourceNow{rev: 2, heads: map[string]string{"novox/mesh-catalog": h2}}
+			ix.Contains = tc.contains
+			var asked []string
+			ix.Read = func(_ context.Context, _, sha, dir string) (RepoRead, error) {
+				asked = append(asked, sha+":"+dir)
+				return RepoRead{Root: "r1", Trees: map[string]string{"modules/a": "treeA2"}, Manifests: map[string]string{"modules/a": `{"module":"a","version":"2"}`}}, nil
+			}
+			got := ix.TakeIn(ctx, "a")
+			m, ok := moduleOf(t, mods, "a")
+			if !tc.moved {
+				if got.Stored || !got.Held || len(asked) != 0 || !strings.Contains(got.Answer, "another head") || !ok || m.Commit != merge {
+					t.Errorf("take-in answered %+v, asked %v, modules.a at %.8s", got, asked, m.Commit)
+				}
+				return
+			}
+			if !got.Stored || !got.Held || got.Commit != h2 || got.Path != "modules/a" || !strings.Contains(got.Answer, "written") {
+				t.Errorf("take-in answered %+v", got)
+			}
+			if !ok || m.Commit != h2 || m.Inputs2 == nil || m.Inputs2.Tree != "treeA2" {
+				t.Errorf("modules.a = %+v (%v), want it at %.8s with tree treeA2 (asked %v)", m, ok, h2, asked)
+			}
+			n := len(asked) // at the head now: a take-in answers held and reads nothing
+			if again := ix.TakeIn(ctx, "a"); again.Stored || !again.Held || len(asked) != n {
+				t.Errorf("a take-in of a module at the source's head answered %+v and asked %v", again, asked)
+			}
+		})
+	}
 }
 
 func mustModule(t *testing.T, m delivery.Module) []byte {
@@ -256,32 +308,3 @@ func TestARecordOfAPullRequestDoesNotReplaceTheRegisteredRef(t *testing.T) { //
 		t.Fatalf("registrations = %+v", got)
 	}
 }
-
-// Issue 1126: mesh-catalog#535 changed mesh-vault, but its merged message never reached the indexer, so the record stood at
-// the merge before. A take-in of the held module reads it again at the head the gate's source names and writes it.
-func TestATakeInOfAHeldModuleBehindTheSourceHeadReadsItAgain(t *testing.T) {
-	ctx := context.Background()
-	ix, mods := modulesIndexer(t)
-	ix.Manifests = newMem()
-	if err := ix.Apply(ctx, mergedMsg("novox/mesh-catalog", map[string]string{"modules/a": "treeA"})); err != nil {
-		t.Fatal(err)
-	}
-	ix.source = sourceNow{rev: 2, heads: map[string]string{"novox/mesh-catalog": h2}}
-	var asked []string
-	ix.Read = func(_ context.Context, _, sha, dir string) (RepoRead, error) {
-		asked = append(asked, sha+":"+dir)
-		return RepoRead{Root: "r1", Trees: map[string]string{"modules/a": "treeA2"}, Manifests: map[string]string{"modules/a": `{"module":"a","version":"2"}`}}, nil
-	}
-	got := ix.TakeIn(ctx, "a")
-	if !got.Stored || !got.Held || got.Commit != h2 || got.Path != "modules/a" || !strings.Contains(got.Answer, "written") {
-		t.Errorf("take-in answered %+v", got)
-	}
-	if m, ok := moduleOf(t, mods, "a"); !ok || m.Commit != h2 || m.Inputs2 == nil || m.Inputs2.Tree != "treeA2" {
-		t.Errorf("modules.a = %+v (%v), want it at %.8s with tree treeA2 (asked %v)", m, ok, h2, asked)
-	}
-	// At the head now: a take-in answers held and reads nothing.
-	n := len(asked)
-	if again := ix.TakeIn(ctx, "a"); again.Stored || !again.Held || len(asked) != n {
-		t.Errorf("a take-in of a module at the source's head answered %+v and asked %v", again, asked)
-	}
-}