Fingerprints computed again by hand run a merge's recompute; the tests join existing ones (issue 1966, review round 1)

Claude-Session: https://claude.ai/code/session_01NcbgfFSnn24M3GcqttoU7i

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

@@ -150,14 +150,15 @@ type Indexer struct {
 	retrying   atomic.Bool                                                       // a RetryUnread runs in the background (kickRetry)
 	retryAgain atomic.Bool                                                       // an event asked for one more RetryUnread after the running one
 	retryWG    sync.WaitGroup                                                    // the background RetryUnread, for tests to wait on
-	fresh      map[string]bool                                                   // modules whose import form is computed again by hand (FingerprintsAgain), "*" for every one; under recompute
-	recompute  sync.Mutex                                                        // one Recompute at a time
-	untaken    map[string]string                                                 // repository to the latest merge applied while the taken state was missing (taken.go)
-	filling    atomic.Bool                                                       // a FillTaken runs in the background (KickFill)
-	fillAgain  atomic.Bool                                                       // a kick arrived while it ran
-	atHeads    map[string]map[string]meshcatalog.PackagesByModuleValue           // a member head's published Go modules, by repository@head (candidatePublished)
-	saidAt     map[string]string                                                 // the head of a repository whose failed read was said, by repository: said once per head
-	failedAt   map[string]string                                                 // the head of a repository whose tree lacks a directory asked, by repository: not asked again until the head changes
+	fresh      map[string]bool                                                   // modules whose import form is computed again by hand (FingerprintsAgain), "*" for every one; under freshMu
+	freshMu    sync.Mutex
+	recompute  sync.Mutex                                              // one Recompute at a time
+	untaken    map[string]string                                       // repository to the latest merge applied while the taken state was missing (taken.go)
+	filling    atomic.Bool                                             // a FillTaken runs in the background (KickFill)
+	fillAgain  atomic.Bool                                             // a kick arrived while it ran
+	atHeads    map[string]map[string]meshcatalog.PackagesByModuleValue // a member head's published Go modules, by repository@head (candidatePublished)
+	saidAt     map[string]string                                       // the head of a repository whose failed read was said, by repository: said once per head
+	failedAt   map[string]string                                       // the head of a repository whose tree lacks a directory asked, by repository: not asked again until the head changes
 	// OwnBuild answers the fingerprint of the build this program runs, from the node-engine's seat; nil knows none.
 	OwnBuild   func(ctx context.Context) (string, error)
 	underSeats bool       // a dry run inside judge.WithSeats: its judge passes do not hold the seat list (verdicts.go)

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

@@ -40,8 +40,6 @@ func (ix *Indexer) FingerprintsAgain(ctx context.Context, module string) TakenIn
 		out.Answer = "the indexer writes no fingerprints"
 		return out
 	}
-	ix.recompute.Lock()
-	defer ix.recompute.Unlock()
 	before, err := ix.fingerprintsHeld(ctx)
 	if err != nil {
 		out.Answer = "the modules state cannot be read: " + err.Error()
@@ -57,13 +55,22 @@ func (ix *Indexer) FingerprintsAgain(ctx context.Context, module string) TakenIn
 	if module == "all" {
 		key = "*"
 	}
+	ix.freshMu.Lock()
 	ix.fresh = map[string]bool{key: true}
-	err = ix.recomputeLocked(ctx, nil, nil)
+	ix.freshMu.Unlock()
+	// The path of a merge's recompute: the modules, then the candidates their fingerprints reach (recomputeWith), then the
+	// pending merges a changed fingerprint settles (PendingAgain, as after a toolchain report).
+	err = ix.recomputeWith(ctx, nil, nil)
+	ix.freshMu.Lock()
 	ix.fresh = nil
+	ix.freshMu.Unlock()
 	if err != nil {
 		out.Answer = "nothing was written: " + err.Error()
 		return out
 	}
+	if err := ix.PendingAgain(ctx); err != nil {
+		ix.logf("the pending merges are not evaluated again after the fingerprints were computed again: %v", err)
+	}
 	after, err := ix.fingerprintsHeld(ctx)
 	if err != nil {
 		out.Answer = "computed, but the modules state cannot be read back: " + err.Error()
@@ -82,6 +89,13 @@ func (ix *Indexer) FingerprintsAgain(ctx context.Context, module string) TakenIn
 	return out
 }
 
+// isFresh says the module's import form is computed again by hand in this pass.
+func (ix *Indexer) isFresh(module string) bool {
+	ix.freshMu.Lock()
+	defer ix.freshMu.Unlock()
+	return ix.fresh[module] || ix.fresh["*"]
+}
+
 // fingerprintsHeld is each held module's fingerprint of the new form.
 func (ix *Indexer) fingerprintsHeld(ctx context.Context) (map[string]string, error) {
 	keys, err := ix.Modules.Keys(ctx)
@@ -113,7 +127,7 @@ func FingerprintsTools(ask func(key string, value []byte) (TakeInAsk, error), no
 			"the import form is written for every module whose imports are all published, whatever fingerprint the module holds. For the " +
 			"fingerprints a pass wrote while the packages state was unread (the version line over the import form: builds then fail on an " +
 			"older package). Answers each module whose fingerprint moved, old and new; when the packages state cannot be read it writes " +
-			"nothing and says why. Acts first, then records the hand act through mesh-hand-acts; when that cannot be recorded the answer says so.",
+			"nothing and says why. Runs as a merge's recompute does (the modules, the candidates they reach, the pending merges they settle). Acts first, then records the hand act through mesh-hand-acts; when that cannot be recorded the answer says so.",
 		Input: map[string]any{"type": "object", "required": []string{"why"}, "properties": map[string]any{
 			"module": map[string]any{"type": "string", "description": "the module's name, or all (the default)"},
 			"why":    map[string]any{"type": "string", "description": "why it is computed again now, in plain words"}}},

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

@@ -1,45 +0,0 @@
-package index
-
-import (
-	"errors"
-	"strings"
-	"testing"
-	"time"
-)
-
-// indexer_fingerprints_again acts first and records the hand act after: a record that fails, or never answers, is said in
-// the answer and never refuses or holds the recovery (mesh-hand-acts on a mesh with no ledger answers nothing).
-func TestFingerprintsAgainActsBeforeItRecordsAndSaysAFailedRecord(t *testing.T) {
-	for _, c := range []struct {
-		name   string
-		record func(what, why string) error
-		said   string
-	}{
-		{"recorded", func(string, string) error { return nil }, ""},
-		{"record refused", func(string, string) error { return errors.New("no ledger") }, "not recorded: no ledger"},
-		{"record never answers", func(string, string) error { select {} }, "not recorded within"},
-	} {
-		t.Run(c.name, func(t *testing.T) {
-			var asked []string
-			ask := func(key string, _ []byte) (TakeInAsk, error) {
-				asked = append(asked, key)
-				return TakeInAsk{Result: &TakenIn{Module: "all", Stored: true}}, nil
-			}
-			tool := FingerprintsTools(ask, time.Now, c.record, 10*time.Millisecond)[0]
-			if _, err := tool.Run(map[string]any{"module": "avahi"}); err == nil {
-				t.Fatal("a hand act with no why was done")
-			}
-			if len(asked) != 0 {
-				t.Fatalf("asked the indexer without a why: %v", asked)
-			}
-			got, err := tool.Run(map[string]any{"module": "avahi", "why": "the packages read came back empty"})
-			if err != nil || len(asked) != 1 || !strings.HasPrefix(asked[0], "fingerprints.avahi.") {
-				t.Fatalf("run: %v, asked %v", err, asked)
-			}
-			said, _ := got.(map[string]any)["hand-act"].(string)
-			if !strings.Contains(said, c.said) || (c.said == "") != (said == "") {
-				t.Fatalf("hand-act %q, want one saying %q", said, c.said)
-			}
-		})
-	}
-}

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

@@ -9,6 +9,7 @@ import (
 
 	"git.novox.be/novox/mesh-delivery/contracts/go/clients/meshcatalogclient"
 	"git.novox.be/novox/mesh-delivery/contracts/go/seats/meshcatalog"
+	"git.novox.be/novox/mesh-delivery/contracts/go/seats/meshdeliverychecker"
 	"git.novox.be/novox/mesh-delivery/modules/mesh-delivery-builder/builds"
 )
 
@@ -52,6 +53,7 @@ func (m *missingDir) Error() string { return "no directory " + m.dir }
 func TestImportsReportedMoveTheFingerprintOnlyWithAnImportedPackage(t *testing.T) {
 	t.Run("a build of an older fingerprint keeps no imports", buildOfAnOlderFingerprintKeepsNoImports)
 	t.Run("a change of a sibling directory alone moves its importer", aSiblingDirectoryAloneMovesItsImporter)
+	t.Run("an unread packages state writes no fingerprint, and they are computed again by hand (issue 1966)", anUnreadPackagesStateWritesNoFingerprintAndFingerprintsAreComputedAgain)
 	t.Run("an accepted series' newest published version pins it, by that version's contents", anAcceptedSeriesPinsItsNewest)
 	ctx := context.Background()
 	ix, mods := newFormIndexer(t, mod{"avahi", catalogueManifest(t, "avahi")})
@@ -276,7 +278,7 @@ func anAcceptedSeriesPinsItsNewest(t *testing.T) {
 // answered no published package where held modules were computed from published ones, writes no fingerprint (the version
 // line over every import form is what an empty state computes); a version line written leaves no import form behind
 // that a later pass takes for the build's; and indexer_fingerprints_again computes the import form again by hand.
-func TestAnUnreadPackagesStateWritesNoFingerprintAndFingerprintsAreComputedAgain(t *testing.T) {
+func anUnreadPackagesStateWritesNoFingerprintAndFingerprintsAreComputedAgain(t *testing.T) {
 	ctx := context.Background()
 	// avahi at the import form, pinned to mesh-sdk at c3, as the test above leaves it.
 	setup := func(t *testing.T) (*Indexer, *memStore, meshcatalog.ModulesByModuleValue) {
@@ -359,6 +361,11 @@ func TestAnUnreadPackagesStateWritesNoFingerprintAndFingerprintsAreComputedAgain
 		if now, _ := moduleOf(t, mods, "avahi"); now.Fp2 != good.Fp2 || ix.fresh != nil {
 			t.Fatalf("avahi holds %q after it, want %q (fresh %v)", now.Fp2, good.Fp2, ix.fresh)
 		}
+		// It runs a merge's recompute, candidates included: a candidate store that cannot be listed fails it, said.
+		ix.Store, ix.previews = &memStore{keysErr: errors.New("no bus")}, map[string]meshdeliverychecker.PreviewsByRepositoryHeadTrunkValue{"k": {}}
+		if r := ix.FingerprintsAgain(ctx, "avahi"); r.Stored || !strings.Contains(r.Answer, "listing the candidates") {
+			t.Fatalf("fingerprints again over candidates that cannot be listed: %+v", r)
+		}
 		if r := ix.FingerprintsAgain(ctx, "nothing"); r.Stored {
 			t.Fatalf("fingerprints again of a module not held answered stored: %+v", r)
 		}

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

@@ -135,6 +135,40 @@ func sha(c byte) string { return strings.Repeat(string(c), 40) }
 
 func TestTakeIn(t *testing.T) { // 1585/T3-T13 (ADR 0633)
 	ctx := context.Background()
+	t.Run("indexer_fingerprints_again acts before it records the hand act and says a failed record", func(t *testing.T) { // issue 1966
+		for _, c := range []struct {
+			name   string
+			record func(what, why string) error
+			said   string
+		}{
+			{"recorded", func(string, string) error { return nil }, ""},
+			{"record refused", func(string, string) error { return errors.New("no ledger") }, "not recorded: no ledger"},
+			{"record never answers", func(string, string) error { select {} }, "not recorded within"},
+		} {
+			t.Run(c.name, func(t *testing.T) {
+				var asked []string
+				ask := func(key string, _ []byte) (TakeInAsk, error) {
+					asked = append(asked, key)
+					return TakeInAsk{Result: &TakenIn{Module: "all", Stored: true}}, nil
+				}
+				tool := FingerprintsTools(ask, time.Now, c.record, 10*time.Millisecond)[0]
+				if _, err := tool.Run(map[string]any{"module": "avahi"}); err == nil {
+					t.Fatal("a hand act with no why was done")
+				}
+				if len(asked) != 0 {
+					t.Fatalf("asked the indexer without a why: %v", asked)
+				}
+				got, err := tool.Run(map[string]any{"module": "avahi", "why": "the packages read came back empty"})
+				if err != nil || len(asked) != 1 || !strings.HasPrefix(asked[0], "fingerprints.avahi.") {
+					t.Fatalf("run: %v, asked %v", err, asked)
+				}
+				said, _ := got.(map[string]any)["hand-act"].(string)
+				if !strings.Contains(said, c.said) || (c.said == "") != (said == "") {
+					t.Fatalf("hand-act %q, want one saying %q", said, c.said)
+				}
+			})
+		}
+	})
 	t.Run("T6 a merge while the own build is unknown writes nothing", func(t *testing.T) {
 		b := newTakeInBed(t)
 		b.ix.ownFP, b.ix.ownFixed = "", false // this process is started again and asks the engine, which does not answer

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

@@ -264,7 +264,7 @@ func (ix *Indexer) recomputeLocked(ctx context.Context, overlay map[string]meshc
 		chain = append(chain, module)
 		kept := *k.cur.Inputs2
 		kept.Bases, kept.Builder = nil, ""
-		if ix.fresh[module] || ix.fresh["*"] { // indexer_fingerprints_again: the import form is written again, the held one stands for nothing
+		if ix.isFresh(module) { // indexer_fingerprints_again: the import form is written again, the held one stands for nothing
 			kept.ImportForm = ""
 		}
 		var missing []string