From 790653dfde9be67cf38e3a55928c88387ae517f7 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 28 Oct 2026 11:08:56 +0100 Subject: [PATCH] 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. --- .../cmd/mesh-delivery-indexer-tools/main.go | 4 +- .../internal/index/registered.go | 60 ++++++++++++++++++- .../internal/index/registered_test.go | 29 +++++++++ 3 files changed, 91 insertions(+), 2 deletions(-) diff --git a/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer-tools/main.go b/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer-tools/main.go index 2640989..6904086 100644 --- a/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer-tools/main.go +++ b/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer-tools/main.go @@ -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.", diff --git a/modules/mesh-delivery-indexer/internal/index/registered.go b/modules/mesh-delivery-indexer/internal/index/registered.go index e83fb4f..0a78a95 100644 --- a/modules/mesh-delivery-indexer/internal/index/registered.go +++ b/modules/mesh-delivery-indexer/internal/index/registered.go @@ -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 +} diff --git a/modules/mesh-delivery-indexer/internal/index/registered_test.go b/modules/mesh-delivery-indexer/internal/index/registered_test.go index 13eca05..6e3ac1e 100644 --- a/modules/mesh-delivery-indexer/internal/index/registered_test.go +++ b/modules/mesh-delivery-indexer/internal/index/registered_test.go @@ -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) + } +}