Put the lagging-build rows into the existing reach test, and word a build that is not the newest as the build the node waits for (issue 1966)

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

modules/mesh-delivery-view/internal/view/reach.go

@@ -268,7 +268,7 @@ func (m *Model) moduleReach(name string, mod meshcatalog.ModulesByModuleValue, h
 		return mr, noBound, ""
 	}
 	next := mr.Changes[floor-1]
-	words := m.laggingWords(name, floorNode, mr.Changes[floor], next, values[floor], values[floor-1])
+	words := m.laggingWords(name, floorNode, mr.Changes[floor], next, values[floor], values[floor-1], floor == 1)
 	if next.Src == 0 {
 		// The next change's order is not known: the head is held at the floor's own revision, or before every head when that is
 		// not known either.
@@ -283,7 +283,7 @@ func (m *Model) moduleReach(name string, mod meshcatalog.ModulesByModuleValue, h
 // laggingWords says what a node's running build lacks of the module's next change: both builds by fingerprint with their
 // commits, which input moved when the commit is the same (the catalogue holds the module's inputs, not the version a mesh
 // package resolved to), and a next build that failed, with the first words of its error, instead of "not yet".
-func (m *Model) laggingWords(name, node string, run, next viewstate.Change, runV, nextV meshcatalog.ModulesByModuleValue) string {
+func (m *Model) laggingWords(name, node string, run, next viewstate.Change, runV, nextV meshcatalog.ModulesByModuleValue, newest bool) string {
 	words := fmt.Sprintf("%s on %s runs build %s (%.8s)", name, node, shortFP(run.FP), run.Commit)
 	nextIs := fmt.Sprintf("%s (%.8s)", shortFP(next.FP), next.Commit)
 	if run.Commit == next.Commit {
@@ -293,7 +293,11 @@ func (m *Model) laggingWords(name, node string, run, next viewstate.Change, runV
 	}
 	if key, err := builds.BuilderBuildKey(name, next.FP); err == nil {
 		if k, ok := m.keys[key]; ok && k.err == "" && k.value.State == builds.BuildFailed {
-			return fmt.Sprintf("%s, its newest build %s failed: %s", words, nextIs, firstWords(k.value.Error))
+			which := "the build it waits for"
+			if newest {
+				which = "its newest build"
+			}
+			return fmt.Sprintf("%s, %s %s failed: %s", words, which, nextIs, firstWords(k.value.Error))
 		}
 	}
 	return fmt.Sprintf("%s, not yet %s", words, nextIs)

modules/mesh-delivery-view/internal/view/reach_test.go

@@ -12,6 +12,7 @@ import (
 	"git.novox.be/novox/mesh-delivery/contracts/go/clients/meshwantsclient"
 	"git.novox.be/novox/mesh-delivery/contracts/go/seats/meshcatalog"
 	"git.novox.be/novox/mesh-delivery/contracts/go/seats/meshdelivery"
+	"git.novox.be/novox/mesh-delivery/modules/mesh-delivery-builder/builds"
 	"git.novox.be/novox/mesh-delivery/modules/mesh-delivery-indexer/takein"
 	"git.novox.be/novox/mesh-delivery/modules/mesh-delivery-view/viewstate"
 	"git.novox.be/novox/mesh-delivery/sdk/trunk"
@@ -31,6 +32,22 @@ func TestTheViewDerivesHowFarEachRepositoryReached(t *testing.T) {
 		}
 		return Entry{Bucket: ModulesBucket, Key: module, Value: v, At: t0, History: history}
 	}
+	modIn := func(fp, commit string, src uint64, in *meshcatalog.ModulesByModuleValueInputs2, history bool) Entry {
+		v, err := meshcatalogclient.EncodeModule(meshcatalog.ModulesByModuleValue{Fp2: fp, Manifest: strings.Repeat("a", 64), Repository: web, Commit: commit, Src: int64(src), Path: "web", Inputs2: in})
+		if err != nil {
+			t.Fatal(err)
+		}
+		return Entry{Bucket: ModulesBucket, Key: "web", Value: v, At: t0, History: history}
+	}
+	in := func(tree string, pkgs ...string) *meshcatalog.ModulesByModuleValueInputs2 {
+		return &meshcatalog.ModulesByModuleValueInputs2{Tree: tree, Packages: pkgs}
+	}
+	// A later build of web (fpLater) that g14, running fpNew, lacks; issue 1966: the hold names both builds, what moved when the
+	// commit is the same, and a build that failed as failed.
+	const fpLater = "src2:9999999999999999999999999999999999999999999999999999999999999999"
+	later := func(commit string, to *meshcatalog.ModulesByModuleValueInputs2, extra ...Entry) []Entry {
+		return append([]Entry{modIn(fpLater, commit, 12, to, true), modIn(fpLater, commit, 12, to, false)}, extra...)
+	}
 	src := func(rev uint64, webHead string) Entry {
 		v, err := meshdeliveryclient.EncodeSource(meshdelivery.SourceByMainValue{Group: "g", Heads: map[string]string{web: webHead, sdk: commitS}})
 		if err != nil {
@@ -58,8 +75,8 @@ func TestTheViewDerivesHowFarEachRepositoryReached(t *testing.T) {
 		current := Entry{Bucket: SourceBucket, Key: meshdeliveryclient.SourceMainKey, Revision: 12, At: t0}
 		current.Value, _ = meshdeliveryclient.EncodeSource(cur)
 		return []Entry{src(10, commitA), src(11, commitB), src(12, commitC), current,
-			mod("web", fpOld, commitA, 10, true), mod("web", fpNew, commitC, 12, true), mod("mail", fpMail, commitA, 0, true),
-			mod("web", fpNew, commitC, 12, false), mod("mail", fpMail, commitA, 0, false),
+			mod("web", fpOld, commitA, 10, true), modIn(fpNew, commitC, 12, in("t1", "mesh-sdk/go"), true), mod("mail", fpMail, commitA, 0, true),
+			modIn(fpNew, commitC, 12, in("t1", "mesh-sdk/go"), false), mod("mail", fpMail, commitA, 0, false),
 			taken(web, commitC), taken(sdk, commitS),
 			wantE(t, "ace", "web", meshwantsclient.Want{Fp: fpNew}), haveFP(t, "ace", "web", fpNew, nodeengineclient.HealthOK),
 			wantE(t, "g14", "web", meshwantsclient.Want{Fp: fpNew}), haveFP(t, "g14", "web", fpNew, nodeengineclient.HealthOK),
@@ -84,6 +101,20 @@ func TestTheViewDerivesHowFarEachRepositoryReached(t *testing.T) {
 			then: []Entry{haveFP(t, "g14", "web", fpOld, nodeengineclient.HealthOK), presenceE(t, "g14", false, t0), presenceE(t, "g14", true, t0)}},
 		{name: "a want held back at an older build holds the head as a lag does", reached: commitB, holds: "web on g14 runs build",
 			then: []Entry{wantE(t, "g14", "web", meshwantsclient.Want{Fp: fpOld, Newest: fpNew, Rule: 2}), haveFP(t, "g14", "web", fpOld, nodeengineclient.HealthOK)}},
+		{name: "a later build of the same commit names the mesh package that moved", reached: commitB,
+			holds: "web on ace runs build 22222222 (cccccccc), not yet 99999999 (cccccccc, a mesh package of mesh-sdk/go)",
+			then:  later(commitC, in("t1", "mesh-sdk/go"))},
+		{name: "a later build of the same commit names the tree that moved", reached: commitB,
+			holds: "runs build 22222222 (cccccccc), not yet 99999999 (cccccccc, tree)", then: later(commitC, in("t2", "mesh-sdk/go"))},
+		{name: "a later build of another commit names no input", reached: commitB,
+			holds: "runs build 22222222 (cccccccc), not yet 99999999 (eeeeeeee)", then: later(commitX, in("t2"))},
+		{name: "the newest build, failed, is said failed with its first words", reached: commitB,
+			holds: "its newest build 99999999 (eeeeeeee) failed: go build ./...: undefined: Foo", then: later(commitX, in("t2"),
+				builderKey(t, "web", fpLater, builds.BuilderBuild{State: builds.BuildFailed, Error: "go build ./...: undefined: Foo"}))},
+		{name: "a failed build that is not the newest is the build the node waits for", reached: commitB,
+			holds: "the build it waits for 99999999 (eeeeeeee) failed: go build", then: append(later(commitX, in("t2"),
+				builderKey(t, "web", fpLater, builds.BuilderBuild{State: builds.BuildFailed, Error: "go build ./...: undefined: Foo"})),
+				modIn("src2:"+strings.Repeat("8", 64), commitD, 12, in("t3"), true), modIn("src2:"+strings.Repeat("8", 64), commitD, 12, in("t3"), false))},
 		{name: "a failing health check is said and holds nothing", reached: commitC, waiting: "health check failed",
 			then: []Entry{haveFP(t, "g14", "web", fpNew, nodeengineclient.HealthFailed)}},
 		{name: "a node with no running have holds no head back and its module's merges are not yet", reached: commitC, waiting: "not running",

modules/mesh-delivery-view/internal/view/reach_words_test.go

@@ -1,77 +0,0 @@
-package view
-
-import (
-	"strings"
-	"testing"
-
-	"git.novox.be/novox/mesh-contracts/go/clients/nodeengineclient"
-	"git.novox.be/novox/mesh-delivery/contracts/go/clients/meshcatalogclient"
-	"git.novox.be/novox/mesh-delivery/contracts/go/clients/meshdeliveryclient"
-	"git.novox.be/novox/mesh-delivery/contracts/go/clients/meshwantsclient"
-	"git.novox.be/novox/mesh-delivery/contracts/go/seats/meshcatalog"
-	"git.novox.be/novox/mesh-delivery/contracts/go/seats/meshdelivery"
-	"git.novox.be/novox/mesh-delivery/modules/mesh-delivery-builder/builds"
-	"git.novox.be/novox/mesh-delivery/modules/mesh-delivery-indexer/takein"
-	"git.novox.be/novox/mesh-delivery/sdk/trunk"
-)
-
-// Issue 1966: a node that runs an older build of a module than its newest names both builds by fingerprint with their commits,
-// says which input moved when the commit is the same, and says a newest build that failed as failed, with its first words.
-func TestALaggingNodeNamesTheBuildsAndWhatDiffers(t *testing.T) {
-	const web = "novox/web"
-	commitA, commitB, commitC := strings.Repeat("a", 40), strings.Repeat("b", 40), strings.Repeat("c", 40)
-	inputs := func(tree string, pkgs ...string) *meshcatalog.ModulesByModuleValueInputs2 {
-		return &meshcatalog.ModulesByModuleValueInputs2{Tree: tree, Packages: pkgs}
-	}
-	for _, tc := range []struct {
-		name         string
-		oldIn, newIn *meshcatalog.ModulesByModuleValueInputs2
-		newCommit    string
-		failed       string
-		want, not    string
-	}{
-		{name: "same commit, a mesh package moved", oldIn: inputs("t", "mesh-sdk/go"), newIn: inputs("t", "mesh-sdk/go"), newCommit: commitA,
-			want: "web on g14 runs build 11111111 (aaaaaaaa), not yet 22222222 (aaaaaaaa, a mesh package of mesh-sdk/go)", not: "the build of"},
-		{name: "same commit, its tree moved", oldIn: inputs("t1"), newIn: inputs("t2"), newCommit: commitA,
-			want: "runs build 11111111 (aaaaaaaa), not yet 22222222 (aaaaaaaa, tree)"},
-		{name: "another commit names no input", oldIn: inputs("t1"), newIn: inputs("t2"), newCommit: commitB,
-			want: "runs build 11111111 (aaaaaaaa), not yet 22222222 (bbbbbbbb)", not: "tree"},
-		{name: "a newest build that failed is said failed", oldIn: inputs("t1"), newIn: inputs("t2"), newCommit: commitB, failed: "go build ./...: undefined: Foo",
-			want: "its newest build 22222222 (bbbbbbbb) failed: go build ./...: undefined: Foo", not: "not yet"},
-	} {
-		t.Run(tc.name, func(t *testing.T) {
-			mod := func(fp, commit string, src int64, in *meshcatalog.ModulesByModuleValueInputs2, history bool) Entry {
-				v, err := meshcatalogclient.EncodeModule(meshcatalog.ModulesByModuleValue{Fp2: fp, Manifest: strings.Repeat("a", 64), Repository: web, Commit: commit, Src: src, Path: "web", Inputs2: in})
-				if err != nil {
-					t.Fatal(err)
-				}
-				return Entry{Bucket: ModulesBucket, Key: "web", Value: v, At: t0, History: history}
-			}
-			src := func(rev uint64, head string, history bool) Entry {
-				v, _ := meshdeliveryclient.EncodeSource(meshdelivery.SourceByMainValue{Group: "g", Heads: map[string]string{web: head}})
-				return Entry{Bucket: SourceBucket, Key: meshdeliveryclient.SourceMainKey, Value: v, At: t0, Revision: rev, History: history}
-			}
-			key, _ := trunk.RepositoryKey(web)
-			v, _ := takein.EncodeTaken(takein.Taken{Repository: web, Merge: commitC, At: t0})
-			m := New()
-			m.SetReachWriter(func(ReachWrite) error { return nil })
-			m.SetDeliveredEvent(func(DeliveredEvent) error { return nil })
-			entries := []Entry{src(10, commitA, true), src(11, tc.newCommit, true), src(12, commitC, true), src(12, commitC, false),
-				mod(fpOld, commitA, 10, tc.oldIn, true), mod(fpNew, tc.newCommit, 11, tc.newIn, true), mod(fpNew, tc.newCommit, 11, tc.newIn, false),
-				{Bucket: TakenBucket, Key: key, Value: v, At: t0},
-				wantE(t, "g14", "web", meshwantsclient.Want{Fp: fpOld}), haveFP(t, "g14", "web", fpOld, nodeengineclient.HealthOK)}
-			if tc.failed != "" {
-				entries = append(entries, builderKey(t, "web", fpNew, builds.BuilderBuild{State: builds.BuildFailed, Error: tc.failed}))
-			}
-			fill(m, entries...)
-			for _, b := range reachBuckets {
-				m.Initial(b)
-			}
-			fill(m, haveFP(t, "g14", "web", fpOld, nodeengineclient.HealthOK))
-			got := strings.Join(m.Reach()[key].Holds, "; ")
-			if !strings.Contains(got, tc.want) || (tc.not != "" && strings.Contains(got, tc.not)) {
-				t.Errorf("holds = %q, want words with %q and none with %q", got, tc.want, tc.not)
-			}
-		})
-	}
-}