From 289ba69da63cc19f72469b81939b43f32082b289 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 6 Nov 2026 10:21:42 +0100 Subject: [PATCH] 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 | 14 ++++++++++---- modules/mesh-delivery-builder/builds/log_test.go | 14 +++++++++++--- .../cmd/mesh-delivery-builder-tools/main.go | 3 ++- .../internal/build/imports_test.go | 5 +++++ .../mesh-delivery-builder/internal/build/run.go | 1 + 5 files changed, 29 insertions(+), 8 deletions(-) diff --git a/modules/mesh-delivery-builder/builds/log.go b/modules/mesh-delivery-builder/builds/log.go index 8bdc55e8..6bf530c5 100644 --- a/modules/mesh-delivery-builder/builds/log.go +++ b/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 diff --git a/modules/mesh-delivery-builder/builds/log_test.go b/modules/mesh-delivery-builder/builds/log_test.go index 4f62c5e0..93de78fc 100644 --- a/modules/mesh-delivery-builder/builds/log_test.go +++ b/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 { diff --git a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder-tools/main.go b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder-tools/main.go index dc3ee1ba..cc24d6ec 100644 --- a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder-tools/main.go +++ b/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"}}}, diff --git a/modules/mesh-delivery-builder/internal/build/imports_test.go b/modules/mesh-delivery-builder/internal/build/imports_test.go index 6f81e2fe..20ee96e5 100644 --- a/modules/mesh-delivery-builder/internal/build/imports_test.go +++ b/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) + } }) } } diff --git a/modules/mesh-delivery-builder/internal/build/run.go b/modules/mesh-delivery-builder/internal/build/run.go index fa76ab4e..4d38f634 100644 --- a/modules/mesh-delivery-builder/internal/build/run.go +++ b/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 }