The forge bridge keeps the directories a failed try read whole, so a large merge converges over its tries (issue 1126)

Without it the first merge after the bridge starts, with nothing kept, still reads all 140
catalogue directories in one 30 s try and times out as before.

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

@@ -203,6 +203,7 @@ const MaxReadsInFlight = 8
 type read struct {
 	dir, path string
 	manifest  bool
+	asked     bool
 	raw       []byte
 	err       error
 }
@@ -216,6 +217,14 @@ type read struct {
 // order the calls finish in, and the error is that of the first failing file in the order of dirs and LockFiles. After
 // a failure no further file is asked for, and inputsOf returns only when every call it started has returned.
 func inputsOf(owner, repo, ref string, dirs []string, files Files) (manifests map[string]string, locks map[string]map[string]string, err error) {
+	manifests, locks, _, err = inputsRead(owner, repo, ref, dirs, files)
+	return manifests, locks, err
+}
+
+// inputsRead is inputsOf, and also answers, on a failure too, the directories whose manifest and lock files were all
+// read, with their manifests and lock hashes: a merge keeps them by their tree, so its next try reads only the rest
+// (issue 1126).
+func inputsRead(owner, repo, ref string, dirs []string, files Files) (manifests map[string]string, locks map[string]map[string]string, whole []string, err error) {
 	at := func(dir, name string) string {
 		if dir == "" {
 			return name
@@ -244,6 +253,7 @@ func inputsOf(owner, repo, ref string, dirs []string, files Files) (manifests ma
 			<-slots
 			break
 		}
+		r.asked = true
 		wg.Add(1)
 		go func(r *read) {
 			defer func() { <-slots; wg.Done() }()
@@ -258,10 +268,16 @@ func inputsOf(owner, repo, ref string, dirs []string, files Files) (manifests ma
 	wg.Wait()
 	manifests = map[string]string{}
 	locks = map[string]map[string]string{}
+	broken := map[string]bool{} // a directory with a file not read (failed, or never asked after a failure)
 	for _, r := range reads {
 		switch {
 		case r.err != nil && (r.manifest || !errors.Is(r.err, ErrNotFound)):
-			return nil, nil, fmt.Errorf("%s: %w", r.path, r.err)
+			broken[r.dir] = true
+			if err == nil {
+				err = fmt.Errorf("%s: %w", r.path, r.err)
+			}
+		case r.err == nil && r.raw == nil && !r.asked:
+			broken[r.dir] = true
 		case r.err != nil: // a lock file the forge does not have
 		case r.manifest:
 			manifests[r.dir] = string(r.raw)
@@ -273,13 +289,25 @@ func inputsOf(owner, repo, ref string, dirs []string, files Files) (manifests ma
 			locks[r.dir][path.Base(r.path)] = hex.EncodeToString(sum[:])
 		}
 	}
+	for _, dir := range dirs {
+		if !broken[dir] {
+			whole = append(whole, dir)
+		}
+	}
+	if err != nil {
+		for dir := range broken {
+			delete(manifests, dir)
+			delete(locks, dir)
+		}
+		return manifests, locks, whole, err
+	}
 	if len(manifests) == 0 {
 		manifests = nil
 	}
 	if len(locks) == 0 {
 		locks = nil
 	}
-	return manifests, locks, nil
+	return manifests, locks, whole, nil
 }
 
 // merged is the merged message of a merge into the trunk: the trees of the merge commit, asked once.
@@ -301,8 +329,9 @@ func merged(subject string, p pull, trees Trees, files Files, list Listed, kept
 	}
 	slices.Sort(names)
 	manifests, locks, toRead := keptInputs(names, dirs, kept)
-	read, readLocks, err := inputsOf(p.Owner, p.Repo, p.MergeSHA, toRead, files)
+	read, readLocks, whole, err := inputsRead(p.Owner, p.Repo, p.MergeSHA, toRead, files)
 	if err != nil {
+		keep(whole, dirs, read, readLocks, kept) // what was read whole is not asked again at the next try
 		return Message{}, fmt.Errorf("%w: %s: the manifests of %s at the merge commit %s (%d of %d module directories read from the forge, the others kept by their tree): %v", ErrRetry, subject, repository, p.MergeSHA, len(toRead), len(names), err)
 	}
 	keep(toRead, dirs, read, readLocks, kept)

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

@@ -416,13 +416,26 @@ func TestAMergeReadsOnlyTheDirectoriesWhoseTreeChanged(t *testing.T) {
 	if string(first.Data) == string(next.Data) {
 		t.Error("the second merge's message did not change")
 	}
-	// A failed read keeps nothing of the directories it read, so the next try reads them again.
+	// A failed read keeps the directories it read whole and nothing of the one that failed, so the next try reads only the
+	// rest: a merge too large for one try's bound converges over its tries.
 	kept2 := memKept{}
 	failing := &forgeFiles{have: forge.have, errs: map[string]error{"modules/b/module.json": errors.New("context deadline exceeded")}}
 	if _, err := ConvertKept(mergedSubject, mergedBody(nil), treesOf(&calls, "r1", trees, nil), failing.get, nil, kept2); !errors.Is(err, ErrRetry) {
 		t.Fatalf("err = %v, want ErrRetry", err)
 	}
-	if len(kept2) != 0 {
-		t.Errorf("a failed read kept %v", kept2)
+	if _, ok := kept2["t1"]; !ok || len(kept2) != 1 { // modules/a's files are asked first, before the failure is seen
+		t.Errorf("a failed read kept %v, want modules/a's tree t1 alone", kept2)
+	}
+	failing.calls = nil
+	failing.errs = nil
+	again, err := ConvertKept(mergedSubject, mergedBody(nil), treesOf(&calls, "r1", trees, nil), failing.get, nil, kept2)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if string(again.Data) != string(whole.Data) {
+		t.Errorf("the message of the try after a failure differs from one read whole")
+	}
+	if len(failing.calls) != 3 {
+		t.Errorf("the try after a failure read %d files, want only modules/b's 3", len(failing.calls))
 	}
 }