The builder's log keeps a failed build's error and step at its end, so builder_builds_since never hides why a build failed (issue 1921)

LogEnded copied the outcome, manifest, bases, artifacts and imports of the build's key but not its error or step, and the log entry had no field for them: a failed build read back as only "failed".

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

modules/mesh-delivery-builder/builds/log.go

@@ -72,6 +72,11 @@ type BuildLogEntry struct {
 	// writes, as the build module answered them (its test step's `out-of-step`): the indexer raises a finding naming them
 	// for a trunk build, and only a clean build of the same module clears it. A start or a narration names none.
 	OutOfStep []string `json:"out-of-step,omitempty"`
+	// Error and Step are, at a build's end, why it failed and the step it was at or did last, as the build's key holds them
+	// (BuilderBuild.Error, Step): a reader of the log sees a failed build's reason, never only its outcome. A start or a
+	// narration names neither.
+	Error string `json:"error,omitempty"`
+	Step  string `json:"step,omitempty"`
 	// Line is a narration's words, Why what it narrates (asked, waiting, stored, refused): a build.said entry's alone.
 	Line string `json:"line,omitempty"`
 	Why  string `json:"why,omitempty"`
@@ -118,8 +123,9 @@ func EncodeBuildLogEntry(e BuildLogEntry) ([]byte, error) {
 		switch {
 		case e.Module == "" || e.FP == "" || e.At.IsZero() || e.Line == "":
 			return nil, fmt.Errorf("builds: a build's narration names its module, fingerprint, line and time")
-		case e.Outcome != "" || len(e.Made) > 0 || e.Manifest != "" || len(e.Against) > 0 || len(e.Imports) > 0 || len(e.OutOfStep) > 0:
-			return nil, fmt.Errorf("builds: a build's narration names no outcome, manifest, base, artifact, import or file out of step")
+		case e.Outcome != "" || len(e.Made) > 0 || e.Manifest != "" || len(e.Against) > 0 || len(e.Imports) > 0 || len(e.OutOfStep) > 0 ||
+			e.Error != "" || e.Step != "":
+			return nil, fmt.Errorf("builds: a build's narration names no outcome, manifest, base, artifact, import, file out of step, error or step")
 		}
 		return json.Marshal(e)
 	case e.Event != BuildStartedEvent && e.Event != BuildEndedEvent:
@@ -131,8 +137,8 @@ func EncodeBuildLogEntry(e BuildLogEntry) ([]byte, error) {
 	case e.Event == BuildEndedEvent && e.Outcome == "":
 		return nil, fmt.Errorf("builds: a build's end names its outcome")
 	case e.Event == BuildStartedEvent && (e.Outcome != "" || len(e.Made) > 0 || e.Manifest != "" || len(e.Against) > 0 || len(e.Imports) > 0 ||
-		len(e.OutOfStep) > 0):
-		return nil, fmt.Errorf("builds: a build's start names no outcome, manifest, base, artifact, import or file out of step")
+		len(e.OutOfStep) > 0 || e.Error != "" || e.Step != ""):
+		return nil, fmt.Errorf("builds: a build's start names no outcome, manifest, base, artifact, import, file out of step, error or step")
 	}
 	if err := trunk.CheckImports(e.Imports); err != nil {
 		return nil, err

modules/mesh-delivery-builder/builds/log_test.go

@@ -13,11 +13,13 @@ func TestTheBuildersLogKeepsAStartAndAnEndAndTheirDuration(t *testing.T) {
 	end := start
 	end.Event, end.At, end.Outcome, end.Manifest, end.Against = BuildEndedEvent, at.Add(3*time.Minute), "done", "sha256:m", []string{"artifact-store://base/image@sha256:b"}
 	end.Made = []BuiltArtifact{{Name: "image", Kind: "image", Digest: "sha256:1", Reference: "artifact-store://letta/image@sha256:1"}}
-	for _, e := range []BuildLogEntry{start, end} {
+	failed := end
+	failed.Outcome, failed.Made, failed.Error, failed.Step = BuildFailed, nil, "go test: TestX failed", "test"
+	for _, e := range []BuildLogEntry{start, end, failed} {
 		raw, err := EncodeBuildLogEntry(e)
 		back, derr := DecodeBuildLogEntry(raw)
 		if err != nil || derr != nil || back.Event != e.Event || back.Outcome != e.Outcome || back.Directory != "modules/letta" ||
-			back.Repository != "novox/mesh-catalog" || back.Commit != "abc123" {
+			back.Repository != "novox/mesh-catalog" || back.Commit != "abc123" || back.Error != e.Error || back.Step != e.Step {
 			t.Errorf("%s read back as %+v, %v %v", e.Event, back, err, derr)
 		}
 	}
@@ -41,7 +43,13 @@ func TestTheBuildersLogKeepsAStartAndAnEndAndTheirDuration(t *testing.T) {
 		t.Error("a module with a dot was taken as a key")
 	}
 	for _, edit := range []func(*BuildLogEntry){func(e *BuildLogEntry) { e.Outcome = "" }, func(e *BuildLogEntry) { e.Commit = "" },
-		func(e *BuildLogEntry) { e.Event, e.Outcome, e.Against = BuildStartedEvent, "", nil }} {
+		func(e *BuildLogEntry) { e.Event, e.Outcome, e.Against = BuildStartedEvent, "", nil },
+		func(e *BuildLogEntry) {
+			e.Event, e.Outcome, e.Against, e.Made, e.Manifest, e.Error = BuildStartedEvent, "", nil, nil, "", "x"
+		},
+		func(e *BuildLogEntry) {
+			e.Event, e.Outcome, e.Against, e.Made, e.Manifest, e.Node, e.Line, e.Step = BuildSaidEvent, "", nil, nil, "", "", "asked", "test"
+		}} {
 		bad := end
 		edit(&bad)
 		if _, err := EncodeBuildLogEntry(bad); err == nil {

modules/mesh-delivery-builder/cmd/mesh-delivery-builder-tools/main.go

@@ -196,7 +196,8 @@ func Tools(r build.StuckReader, askAgain func(key string, value []byte) error, q
 		{Name: builds.BuildsSinceTool,
 			Description: "Every build that ended since a time, oldest first, from the builder's log build-log (kept 30 days): per build its module, " +
 				"fingerprint, source revision, repository, directory, commit and ref, the build node, when it ended, its outcome (done, failed or " +
-				"queued), the manifest built, the bases it was built against and each artifact made with its digest and reference. more says the " +
+				"queued), the manifest built, the bases it was built against and each artifact made with its digest and reference; a failed " +
+				"build carries its error and the step it was at. more says the " +
 				"log held more than one answer carries: ask again from the last build's time. What the operator's command nox builds reads. Read-only.",
 			Input: map[string]any{"type": "object", "required": []string{"since"}, "properties": map[string]any{
 				"since": map[string]any{"type": "string", "description": "RFC 3339 time; the builds that ended at or after it"}}},

modules/mesh-delivery-builder/internal/build/imports_test.go

@@ -74,6 +74,11 @@ func outOfStepFilesABuildAnswersReachTheLog(t *testing.T) {
 			if len(logged) != 2 || strings.Join(logged[1].OutOfStep, ",") != row.want || len(logged[0].OutOfStep) != 0 {
 				t.Fatalf("log %+v, want the end naming %q", logged, row.want)
 			}
+			// A failed build's end carries its reason, so the log never hides why it failed; a passed one no error.
+			if end := logged[1]; row.fail != strings.Contains(end.Error, "out of step") || !row.fail && end.Error != "" ||
+				logged[0].Error != "" || logged[0].Step != "" {
+				t.Errorf("log %+v, want a failed build's end alone to carry its error", logged)
+			}
 		})
 	}
 }

modules/mesh-delivery-builder/internal/build/run.go

@@ -1009,6 +1009,7 @@ func LogEnded(module string, m meshcatalog.ModulesByModuleValue, ref, by string,
 	}
 	e.Made = append([]builds.BuiltArtifact(nil), result.Artifacts...)
 	e.Imports = append([]string(nil), result.Imports...) // the indexer keeps them on the module (novox/hq ADR 0593 §2)
+	e.Error, e.Step = result.Error, result.Step          // a failed build's reason, not only its outcome
 	return e
 }