fix: do not show flashing instructions when build failed

cmd/caib/buildcmd/logs.go

@@ -165,9 +165,8 @@ func (h *Handler) waitForBuildCompletion(ctx context.Context, api *buildapiclien
 					lastPhase == phaseFlashing
 
 				handleErr := fmt.Errorf("%s", st.Message)
-				// Only show push/flash results when an image was actually produced.
 				hasImage := st.DiskImage != "" || st.ContainerImage != ""
-				if hasImage && (isFlashFailure || *h.opts.FlashAfterBuild) {
+				if hasImage && isFlashFailure {
 					h.displayBuildResults(ctx, api, name)
 					h.handleFlashError(handleErr, st)
 				} else {

cmd/caib/buildcmd/logs_test.go

@@ -0,0 +1,280 @@
+package buildcmd
+
+import (
+	"encoding/json"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"strings"
+	"testing"
+
+	buildapitypes "github.com/centos-automotive-suite/automotive-dev-operator/internal/buildapi"
+	buildapiclient "github.com/centos-automotive-suite/automotive-dev-operator/internal/buildapi/client"
+)
+
+func newTestOpts() Options {
+	opts := newTestDiskOpts()
+	var (
+		workspace        string
+		extraRepos       []string
+		flashCmd         string
+		exporterSelector string
+	)
+	opts.Workspace = &workspace
+	opts.ExtraRepos = &extraRepos
+	opts.FlashCmd = &flashCmd
+	opts.ExporterSelector = &exporterSelector
+	return opts
+}
+
+// fakeBuildServer creates an httptest.Server that responds to /v1/builds/<name>
+// with the given BuildResponse sequence. Each call to GetBuild returns the next
+// response; once exhausted it repeats the last one. Progress endpoint returns 404.
+func fakeBuildServer(t *testing.T, responses []buildapitypes.BuildResponse) *httptest.Server {
+	t.Helper()
+	callIdx := 0
+	return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if strings.Contains(r.URL.Path, "/progress") {
+			w.WriteHeader(http.StatusNotFound)
+			return
+		}
+		idx := callIdx
+		if idx < len(responses)-1 {
+			callIdx++
+		}
+		w.Header().Set("Content-Type", "application/json")
+		if err := json.NewEncoder(w).Encode(responses[idx]); err != nil {
+			t.Errorf("failed to encode response: %v", err)
+		}
+	}))
+}
+
+func TestWaitForBuildCompletion_BuildFailedNoDiskImage(t *testing.T) {
+	responses := []buildapitypes.BuildResponse{
+		{
+			Name:    "test-build",
+			Phase:   "Failed",
+			Message: `"step-build-image" exited with code 1`,
+		},
+	}
+	srv := fakeBuildServer(t, responses)
+	defer srv.Close()
+
+	opts := newTestOpts()
+	*opts.ServerURL = srv.URL
+	timeout := 1
+	opts.Timeout = &timeout
+
+	var capturedErr error
+	opts.HandleError = func(err error) { capturedErr = err }
+	h := NewHandler(opts)
+
+	api, err := buildapiclient.New(srv.URL)
+	if err != nil {
+		t.Fatalf("failed to create client: %v", err)
+	}
+
+	// Capture stdout to verify no flash instructions printed
+	old := os.Stdout
+	r, w, _ := os.Pipe()
+	os.Stdout = w
+
+	waitErr := h.waitForBuildCompletion(t.Context(), api, "test-build")
+
+	_ = w.Close()
+	out, _ := io.ReadAll(r)
+	os.Stdout = old
+
+	if waitErr == nil {
+		t.Fatal("expected error from waitForBuildCompletion")
+	}
+
+	output := string(out)
+	if strings.Contains(output, "Flash failed") {
+		t.Errorf("should not show flash instructions when build itself failed, got: %s", output)
+	}
+	if strings.Contains(output, "flash manually") {
+		t.Errorf("should not show manual flash guidance when build failed, got: %s", output)
+	}
+	if capturedErr == nil {
+		t.Fatal("expected HandleError to be called")
+	}
+	if !strings.Contains(capturedErr.Error(), "step-build-image") {
+		t.Errorf("error should contain build failure message, got: %v", capturedErr)
+	}
+}
+
+func TestWaitForBuildCompletion_BuildFailedWithDiskImage_NotFlashFailure(t *testing.T) {
+	// Edge case: server returns DiskImage on a build failure (shouldn't happen
+	// with server fix, but tests client-side defense-in-depth).
+	responses := []buildapitypes.BuildResponse{
+		{
+			Name:      "test-build",
+			Phase:     "Failed",
+			Message:   `"step-build-image" exited with code 1`,
+			DiskImage: "registry.example.com/img:disk",
+		},
+	}
+	srv := fakeBuildServer(t, responses)
+	defer srv.Close()
+
+	opts := newTestOpts()
+	*opts.ServerURL = srv.URL
+	*opts.FlashAfterBuild = true
+	timeout := 1
+	opts.Timeout = &timeout
+
+	var capturedErr error
+	opts.HandleError = func(err error) { capturedErr = err }
+	h := NewHandler(opts)
+
+	api, err := buildapiclient.New(srv.URL)
+	if err != nil {
+		t.Fatalf("failed to create client: %v", err)
+	}
+
+	old := os.Stdout
+	r, w, _ := os.Pipe()
+	os.Stdout = w
+
+	waitErr := h.waitForBuildCompletion(t.Context(), api, "test-build")
+
+	_ = w.Close()
+	out, _ := io.ReadAll(r)
+	os.Stdout = old
+
+	if waitErr == nil {
+		t.Fatal("expected error from waitForBuildCompletion")
+	}
+
+	output := string(out)
+	// Even though DiskImage is set and FlashAfterBuild is true, the message
+	// does not indicate a flash failure, so no flash instructions should appear.
+	if strings.Contains(output, "Flash failed") {
+		t.Errorf("should not show flash instructions for build failure (not flash failure), got: %s", output)
+	}
+	if capturedErr == nil {
+		t.Fatal("expected HandleError to be called")
+	}
+}
+
+func TestWaitForBuildCompletion_FlashFailure_ShowsFlashInstructions(t *testing.T) {
+	responses := []buildapitypes.BuildResponse{
+		{
+			Name:      "test-build",
+			Phase:     "Failed",
+			Message:   "Flash to device failed: timeout waiting for device",
+			DiskImage: "registry.example.com/ns/test-build:disk",
+			Jumpstarter: &buildapitypes.JumpstarterInfo{
+				Available:        true,
+				ExporterSelector: "board-type=renesas-rcar-s4,enabled=true",
+				FlashCmd:         "j storage flash oci://registry.example.com/ns/test-build:disk",
+			},
+		},
+	}
+	srv := fakeBuildServer(t, responses)
+	defer srv.Close()
+
+	opts := newTestOpts()
+	*opts.ServerURL = srv.URL
+	*opts.FlashAfterBuild = true
+	timeout := 1
+	opts.Timeout = &timeout
+
+	var capturedErr error
+	opts.HandleError = func(err error) { capturedErr = err }
+	h := NewHandler(opts)
+
+	api, err := buildapiclient.New(srv.URL)
+	if err != nil {
+		t.Fatalf("failed to create client: %v", err)
+	}
+
+	old := os.Stdout
+	r, w, _ := os.Pipe()
+	os.Stdout = w
+
+	waitErr := h.waitForBuildCompletion(t.Context(), api, "test-build")
+
+	_ = w.Close()
+	out, _ := io.ReadAll(r)
+	os.Stdout = old
+
+	if waitErr == nil {
+		t.Fatal("expected error from waitForBuildCompletion")
+	}
+
+	output := string(out)
+	if !strings.Contains(output, "Flash failed") {
+		t.Errorf("expected flash failure message, got: %s", output)
+	}
+	if !strings.Contains(output, "flash manually") {
+		t.Errorf("expected manual flash guidance, got: %s", output)
+	}
+	if !strings.Contains(output, "j storage flash") {
+		t.Errorf("expected flash command in output, got: %s", output)
+	}
+	if capturedErr == nil {
+		t.Fatal("expected HandleError to be called")
+	}
+	if !strings.Contains(capturedErr.Error(), "Flash to device failed") {
+		t.Errorf("error should contain flash failure message, got: %v", capturedErr)
+	}
+}
+
+func TestWaitForBuildCompletion_FlashFailure_NoJumpstarter(t *testing.T) {
+	// Flash failure detected but no Jumpstarter info — should still call
+	// handleFlashError (which calls handleError) but not print flash instructions.
+	responses := []buildapitypes.BuildResponse{
+		{
+			Name:      "test-build",
+			Phase:     "Failed",
+			Message:   "Flash to device failed: connection refused",
+			DiskImage: "registry.example.com/ns/test-build:disk",
+		},
+	}
+	srv := fakeBuildServer(t, responses)
+	defer srv.Close()
+
+	opts := newTestOpts()
+	*opts.ServerURL = srv.URL
+	*opts.FlashAfterBuild = true
+	timeout := 1
+	opts.Timeout = &timeout
+
+	var capturedErr error
+	opts.HandleError = func(err error) { capturedErr = err }
+	h := NewHandler(opts)
+
+	api, err := buildapiclient.New(srv.URL)
+	if err != nil {
+		t.Fatalf("failed to create client: %v", err)
+	}
+
+	old := os.Stdout
+	r, w, _ := os.Pipe()
+	os.Stdout = w
+
+	waitErr := h.waitForBuildCompletion(t.Context(), api, "test-build")
+
+	_ = w.Close()
+	out, _ := io.ReadAll(r)
+	os.Stdout = old
+
+	if waitErr == nil {
+		t.Fatal("expected error from waitForBuildCompletion")
+	}
+
+	output := string(out)
+	// No Jumpstarter info, so no "flash manually" instructions should appear
+	if strings.Contains(output, "flash manually") {
+		t.Errorf("should not show flash instructions without Jumpstarter info, got: %s", output)
+	}
+	if capturedErr == nil {
+		t.Fatal("expected HandleError to be called")
+	}
+	if !strings.Contains(capturedErr.Error(), "Flash to device failed") {
+		t.Errorf("error should contain flash failure message, got: %v", capturedErr)
+	}
+}

internal/buildapi/server.go

@@ -95,6 +95,19 @@ const (
 	maxManifestSize = 900 * 1024
 )
 
+// buildProducedArtifacts reports whether artifact URLs point to real objects.
+// For Failed builds, only true when flash was attempted (proving push succeeded).
+func buildProducedArtifacts(build *automotivev1alpha1.ImageBuild) bool {
+	switch build.Status.Phase {
+	case phaseCompleted, phaseFlashing:
+		return true
+	case phaseFailed:
+		return build.Status.FlashTaskRunName != ""
+	default:
+		return false
+	}
+}
+
 var getClientFromRequestFn = getClientFromRequest
 var getRESTConfigFromRequestFn = getRESTConfigFromRequest
 var createInternalRegistrySecretFn = createInternalRegistrySecret
@@ -1330,8 +1343,11 @@ func listBuilds(c *gin.Context) {
 			compStr = b.Status.CompletionTime.Format(time.RFC3339)
 		}
 
-		containerImage := b.Spec.GetContainerPush()
-		diskImage := b.Spec.GetExportOCI()
+		var containerImage, diskImage string
+		if buildProducedArtifacts(&b) {
+			containerImage = b.Spec.GetContainerPush()
+			diskImage = b.Spec.GetExportOCI()
+		}
 		if b.Spec.GetUseServiceAccountAuth() && externalRoute != "" {
 			if containerImage != "" {
 				containerImage = translateToExternalURL(containerImage, externalRoute)
@@ -1369,8 +1385,13 @@ func (a *APIServer) getBuild(c *gin.Context, name string) {
 		return
 	}
 
-	containerImage := build.Spec.GetContainerPush()
-	diskImage := build.Spec.GetExportOCI()
+	// Only report artifact URLs when the build progressed past the Building
+	// phase — otherwise no image was produced and the URLs are misleading.
+	var containerImage, diskImage string
+	if buildProducedArtifacts(build) {
+		containerImage = build.Spec.GetContainerPush()
+		diskImage = build.Spec.GetExportOCI()
+	}
 	var warning string
 
 	if build.Spec.GetUseServiceAccountAuth() {

internal/buildapi/server_test.go

@@ -689,4 +689,29 @@ var _ = Describe("APIServer Performance", func() {
 
 		Expect(w.Code).To(Equal(http.StatusOK))
 	})
+
+	Context("buildProducedArtifacts", func() {
+		DescribeTable("returns correct result for each phase",
+			func(phase string, pushTaskRun string, flashTaskRun string, expected bool) {
+				build := &automotivev1alpha1.ImageBuild{
+					Status: automotivev1alpha1.ImageBuildStatus{
+						Phase:            phase,
+						PushTaskRunName:  pushTaskRun,
+						FlashTaskRunName: flashTaskRun,
+					},
+				}
+				Expect(buildProducedArtifacts(build)).To(Equal(expected))
+			},
+			Entry("Pending", phasePending, "", "", false),
+			Entry("Uploading", phaseUploading, "", "", false),
+			Entry("Building", phaseBuilding, "", "", false),
+			Entry("Pushing (in progress)", phasePushing, "", "", false),
+			Entry("Flashing", phaseFlashing, "", "", true),
+			Entry("Completed", phaseCompleted, "", "", true),
+			Entry("Cancelled", phaseCancelled, "", "", false),
+			Entry("Failed during build (no push/flash)", phaseFailed, "", "", false),
+			Entry("Failed during push", phaseFailed, "push-taskrun", "", false),
+			Entry("Failed during flash", phaseFailed, "push-taskrun", "flash-taskrun", true),
+		)
+	})
 })