indexer_take_in reads a held module again at the source's head when its record stands behind it (issue 1126)

A merge whose merged message never reached the indexer had no recovery: take-in answered held for mesh-vault, whose record stayed at 78d7ef7 after mesh-catalog#535.
2026-10-28 11:10:09 +01:00
parent 89be76bb6a
commit 790653dfde
3 changed files with 91 additions and 2 deletions
@@ -29,7 +29,9 @@ func Tools(ask asker, now func() time.Time) []stdio.Tool {
 			Description: "Take a registered module in now: the indexer reads its manifest from the repository and commit the controller's " +
 				"build records register it from (at the head the gate's source names, when it names one), whatever it tried before, and " +
 				"writes it to its modules state as a merge would. For a module registered from outside the indexed repositories whose " +
-				"repository the forge could not read (a 404 before the forge's account could read it). Answers stored, from which " +
+				"repository the forge could not read (a 404 before the forge's account could read it). A module the indexer holds " +
+				"whose record stands behind the head the gate's source names (a merge whose merged message never reached the " +
+				"indexer) is read again at that head and written when it changed. Answers stored, from which " +
 				"repository, commit and path, or why not (held already, no registration, the forge's error, no manifest there). " +
 				"The indexer reads such a module again by itself on the next source revision and merged message; this asks now. " +
 				"indexer_module (mesh-delivery-view) reads the result.",
@@ -45,6 +45,12 @@ type taken struct {
 }

 func (ix *Indexer) indexRegistered(ctx context.Context, r Registration, force bool) (taken, error) {
+	return ix.indexAt(ctx, r, force, false)
+}
+
+// indexAt is indexRegistered; reread also reads a module the modules state holds (a person's take-in of a held module
+// whose record stands behind the head the gate's source names, issue 1126).
+func (ix *Indexer) indexAt(ctx context.Context, r Registration, force, reread bool) (taken, error) {
 	if ix.Modules == nil || ix.Read == nil {
 		return taken{why: "the indexer reads no forge or writes no modules"}, nil
 	}
@@ -55,7 +61,7 @@ func (ix *Indexer) indexRegistered(ctx context.Context, r Registration, force bo
 	}
 	if _, _, found, err := ix.Modules.Get(ctx, key); err != nil {
 		return taken{}, fmt.Errorf("modules.%s: %w", key, err)
-	} else if found {
+	} else if found && !reread {
 		ix.markRead(r.Module)
 		return taken{held: true, why: "the modules state holds it already: a merge to its repository writes it from now on"}, nil
 	}
@@ -204,6 +210,9 @@ type TakenIn struct {
 // controller's records, or whose every registration could not be read or holds no manifest of it, is answered with why.
 func (ix *Indexer) TakeIn(ctx context.Context, module string) TakenIn {
 	out := TakenIn{Module: module}
+	if again, ok := ix.takeInHeld(ctx, module); ok {
+		return again
+	}
 	if ix.Builds == nil {
 		out.Answer = "the indexer reads no controller's build records, so it knows no registration"
 		return out
@@ -273,3 +282,52 @@ func (ix *Indexer) IndexRegisteredAll(ctx context.Context) error {
 	}
 	return first
 }
+
+// takeInHeld reads again, at the head the gate's source names for its repository, a module the modules state holds whose
+// record stands at another commit, and writes it as a merge would (only when it changed). It is the recovery for a merge
+// whose merged message never reached the indexer (issue 1126: mesh-catalog#535's message was given up by the forge bridge
+// after its reads timed out, and mesh-vault's record stayed at the merge before). It answers false where it does not
+// apply: no such module held, or no head named, or the record at that head already.
+func (ix *Indexer) takeInHeld(ctx context.Context, module string) (TakenIn, bool) {
+	out := TakenIn{Module: module}
+	key, err := delivery.ModulesKey(module)
+	if err != nil || ix.Modules == nil {
+		return out, false
+	}
+	value, _, found, err := ix.Modules.Get(ctx, key)
+	if err != nil || !found {
+		return out, false
+	}
+	cur, err := delivery.DecodeModule(value)
+	if err != nil || cur.Repository == "" || cur.Path == "" {
+		return out, false
+	}
+	head := ""
+	ix.mu.Lock()
+	for repo, h := range ix.source.heads {
+		if delivery.SameRepository(repo, cur.Repository) && h != "" {
+			head = h
+		}
+	}
+	ix.mu.Unlock()
+	if head == "" || head == cur.Commit {
+		return out, false
+	}
+	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 {
+	case err != nil:
+		out.Answer = "held, and not read again at the source's head: " + err.Error()
+	case !t.stored:
+		out.Answer = "held, and not read again at the source's head: " + t.why
+	default:
+		after, _, _, _ := ix.Modules.Get(ctx, key)
+		out.Stored = true
+		if string(after) == string(value) {
+			out.Answer = fmt.Sprintf("held, and read again from %s at %s: unchanged there, so its record stands", t.repository, t.commit)
+		} else {
+			out.Answer = fmt.Sprintf("held, read again from %s at %s and written to the modules state: it changed since %.12s", t.repository, t.commit, cur.Commit)
+		}
+	}
+	return out, true
+}
@@ -256,3 +256,32 @@ 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)
+	}
+}