perf(cache): return detailed download results for smarter cache decisions

Changes RemoteCache.Download() to return map[string]DownloadResult instead
of error, providing visibility into individual package download outcomes.

Benefits:
- Distinguishes 'not found' (rebuild required) from 'failed' (retry may help)
- Enables smarter status tracking with PackageDownloadFailed and
  PackageVerificationFailed statuses
- Reduces unnecessary rebuilds by identifying transient failures
- Provides better observability into cache performance

The new DownloadResult type includes:
- DownloadStatusSuccess: Package downloaded successfully
- DownloadStatusNotFound: Package doesn't exist in remote cache
- DownloadStatusFailed: Transient failure (network, timeout, etc.)
- DownloadStatusVerificationFailed: SLSA verification failed
- DownloadStatusSkipped: Package already in local cache

Co-authored-by: Ona <no-reply@ona.com>

Author: leodido

cmd/build.go

@@ -473,8 +473,13 @@ func (c *pushOnlyRemoteCache) ExistingPackages(ctx context.Context, pkgs []cache
 	return c.C.ExistingPackages(ctx, pkgs)
 }
 
-func (c *pushOnlyRemoteCache) Download(ctx context.Context, dst cache.LocalCache, pkgs []cache.Package) error {
-	return nil
+func (c *pushOnlyRemoteCache) Download(ctx context.Context, dst cache.LocalCache, pkgs []cache.Package) map[string]cache.DownloadResult {
+	// Push-only cache doesn't download anything
+	results := make(map[string]cache.DownloadResult)
+	for _, pkg := range pkgs {
+		results[pkg.FullName()] = cache.DownloadResult{Status: cache.DownloadStatusNotFound}
+	}
+	return results
 }
 
 func (c *pushOnlyRemoteCache) Upload(ctx context.Context, src cache.LocalCache, pkgs []cache.Package) error {
@@ -497,7 +502,7 @@ func (c *pullOnlyRemoteCache) ExistingPackages(ctx context.Context, pkgs []cache
 	return c.C.ExistingPackages(ctx, pkgs)
 }
 
-func (c *pullOnlyRemoteCache) Download(ctx context.Context, dst cache.LocalCache, pkgs []cache.Package) error {
+func (c *pullOnlyRemoteCache) Download(ctx context.Context, dst cache.LocalCache, pkgs []cache.Package) map[string]cache.DownloadResult {
 	return c.C.Download(ctx, dst, pkgs)
 }
 
@@ -523,7 +528,7 @@ func parseSLSAConfig(cmd *cobra.Command) (*cache.SLSAConfig, error) {
 	slsaVerificationEnabled := os.Getenv(EnvvarSLSACacheVerification) == "true"
 	slsaSourceURI := os.Getenv(EnvvarSLSASourceURI)
 	requireAttestation := os.Getenv(EnvvarSLSARequireAttestation) == "true"
-	
+
 	// CLI flags override environment variables (if cmd is provided)
 	if cmd != nil {
 		if cmd.Flags().Changed("slsa-cache-verification") {
@@ -542,17 +547,17 @@ func parseSLSAConfig(cmd *cobra.Command) (*cache.SLSAConfig, error) {
 			}
 		}
 	}
-	
+
 	// If verification is disabled, return nil
 	if !slsaVerificationEnabled {
 		return nil, nil
 	}
-	
+
 	// Validation: source URI is required when verification is enabled
 	if slsaSourceURI == "" {
 		return nil, fmt.Errorf("--slsa-source-uri is required when using --slsa-cache-verification")
 	}
-	
+
 	return &cache.SLSAConfig{
 		Verification:       true,
 		SourceURI:          slsaSourceURI,
@@ -564,19 +569,18 @@ func parseSLSAConfig(cmd *cobra.Command) (*cache.SLSAConfig, error) {
 func getRemoteCache(cmd *cobra.Command) cache.RemoteCache {
 	remoteCacheBucket := os.Getenv(EnvvarRemoteCacheBucket)
 	remoteStorage := os.Getenv(EnvvarRemoteCacheStorage)
-	
+
 	// Parse SLSA configuration
 	slsaConfig, err := parseSLSAConfig(cmd)
 	if err != nil {
 		log.Fatalf("SLSA configuration error: %v", err)
 	}
-	
+
 	if remoteCacheBucket != "" {
 		config := &cache.RemoteConfig{
 			BucketName: remoteCacheBucket,
 			SLSA:       slsaConfig,
 		}
-		
 
 		switch remoteStorage {
 		case "GCP":

cmd/sbom-export.go

@@ -190,9 +190,9 @@ func GetPackagePath(pkg *leeway.Package, localCache cache.LocalCache) (packagePa
 				log.Infof("Package %s found in remote cache, downloading...", pkg.FullName())
 
 				// Download the package from the remote cache
-				err := remoteCache.Download(context.Background(), localCache, pkgsToCheck)
-				if err != nil {
-					log.WithError(err).Fatalf("Failed to download package %s from remote cache", pkg.FullName())
+				results := remoteCache.Download(context.Background(), localCache, pkgsToCheck)
+				if result, ok := results[pkg.FullName()]; ok && result.Status == cache.DownloadStatusFailed {
+					log.WithError(result.Err).Fatalf("Failed to download package %s from remote cache", pkg.FullName())
 				}
 
 				// Check if the download was successful

pkg/leeway/build.go

@@ -762,29 +762,54 @@ func Build(pkg *Package, opts ...BuildOption) (err error) {
 		pkgsToDownloadCache[i] = p
 	}
 
-	// Errors are logged but don't fail the build
-	err = ctx.RemoteCache.Download(context.Background(), ctx.LocalCache, pkgsToDownloadCache)
-	if err != nil {
-		// Download errors are already logged by the cache implementation
-		// We continue with local builds for packages that failed to download
-		log.WithError(err).Debug("Remote cache download completed with some errors")
-	}
+	// Download packages and get detailed results for each
+	downloadResults := ctx.RemoteCache.Download(context.Background(), ctx.LocalCache, pkgsToDownloadCache)
 
-	// Update status based on actual outcome - check what's actually in local cache now
+	// Update status based on download results - enables smarter decisions about retries vs rebuilds
+	var failedCount, notFoundCount, successCount int
 	for _, p := range pkgsToDownload {
-		if _, nowInCache := ctx.LocalCache.Location(p); nowInCache {
-			// Successfully downloaded and verified
+		result, hasResult := downloadResults[p.FullName()]
+		_, inCache := ctx.LocalCache.Location(p)
+
+		if inCache {
+			// Successfully downloaded (or was already in cache)
 			pkgstatus[p] = PackageDownloaded
+			successCount++
+		} else if hasResult {
+			switch result.Status {
+			case cache.DownloadStatusNotFound:
+				// Package doesn't exist in remote cache - must build locally
+				pkgstatus[p] = PackageNotBuiltYet
+				notFoundCount++
+			case cache.DownloadStatusVerificationFailed:
+				// SLSA verification failed - must build locally with proper attestation
+				pkgstatus[p] = PackageVerificationFailed
+				log.WithField("package", p.FullName()).Warn("SLSA verification failed, will rebuild locally")
+			case cache.DownloadStatusFailed:
+				// Transient failure - mark for rebuild but log for visibility
+				pkgstatus[p] = PackageDownloadFailed
+				failedCount++
+				log.WithFields(log.Fields{
+					"package": p.FullName(),
+					"error":   result.Err,
+				}).Debug("Download failed (transient), will rebuild locally")
+			default:
+				pkgstatus[p] = PackageNotBuiltYet
+			}
 		} else {
-			// Download failed or verification failed: we will need to build locally
-			// TODO: Distinguish between download failures and verification failures.
-			// Currently can't tell because Download() returns nil on errors (by design for graceful fallback).
-			// To fix: Change RemoteCache.Download() to return map[string]DownloadResult with typed status,
-			// then use PackageDownloadFailed vs PackageVerificationFailed appropriately.
+			// No result - shouldn't happen but handle gracefully
 			pkgstatus[p] = PackageNotBuiltYet
 		}
 	}
 
+	if failedCount > 0 || notFoundCount > 0 {
+		log.WithFields(log.Fields{
+			"success":  successCount,
+			"notFound": notFoundCount,
+			"failed":   failedCount,
+		}).Debug("Download phase completed")
+	}
+
 	// Validate that cached packages have all their dependencies available.
 	// This prevents build failures when a package is cached but a dependency failed to download.
 	// If a cached package has missing dependencies, remove it from cache and mark for rebuild.

pkg/leeway/build_integration_test.go

@@ -2215,20 +2215,26 @@ func (m *mockRemoteCacheWithFailures) ExistingPackages(ctx context.Context, pkgs
 	return result, nil
 }
 
-func (m *mockRemoteCacheWithFailures) Download(ctx context.Context, dst cache.LocalCache, pkgs []cache.Package) error {
+func (m *mockRemoteCacheWithFailures) Download(ctx context.Context, dst cache.LocalCache, pkgs []cache.Package) map[string]cache.DownloadResult {
+	results := make(map[string]cache.DownloadResult)
 	for _, pkg := range pkgs {
 		if _, shouldFail := m.failDownload[pkg.FullName()]; shouldFail {
-			// Simulate download failure - don't copy to local cache
+			// Simulate download failure
+			results[pkg.FullName()] = cache.DownloadResult{
+				Status: cache.DownloadStatusFailed,
+				Err:    fmt.Errorf("simulated download failure"),
+			}
 			continue
 		}
 		if _, exists := m.existingPackages[pkg.FullName()]; exists {
 			// Simulate successful download by creating a dummy cache file
 			m.downloaded[pkg.FullName()] = struct{}{}
-			// Note: We don't actually create files here because we're testing
-			// the validation logic, not the actual download
+			results[pkg.FullName()] = cache.DownloadResult{Status: cache.DownloadStatusSuccess}
+		} else {
+			results[pkg.FullName()] = cache.DownloadResult{Status: cache.DownloadStatusNotFound}
 		}
 	}
-	return nil // Download returns nil even on failures (by design)
+	return results
 }
 
 func (m *mockRemoteCacheWithFailures) Upload(ctx context.Context, src cache.LocalCache, pkgs []cache.Package) error {
@@ -2322,9 +2328,7 @@ func TestDependencyValidation_AfterDownload_Integration(t *testing.T) {
   deps:
   - pkgB:lib
   config:
-    packaging: library
-    dontLint: true
-    dontTest: true`
+    packaging: library`
 	if err := os.WriteFile(filepath.Join(pkgADir, "BUILD.yaml"), []byte(buildYAMLA), 0644); err != nil {
 		t.Fatal(err)
 	}
@@ -2361,9 +2365,7 @@ func Hello() string {
   deps:
   - pkgA:lib
   config:
-    packaging: app
-    dontLint: true
-    dontTest: true`
+    packaging: app`
 	if err := os.WriteFile(filepath.Join(pkgXDir, "BUILD.yaml"), []byte(buildYAMLX), 0644); err != nil {
 		t.Fatal(err)
 	}

pkg/leeway/cache/remote/gsutil.go

@@ -90,9 +90,12 @@ func (rs *GSUtilCache) ExistingPackages(ctx context.Context, pkgs []cache.Packag
 	return existingPackages, nil
 }
 
-// Download makes a best-effort attempt at downloading previously cached build artifacts
-func (rs *GSUtilCache) Download(ctx context.Context, dst cache.LocalCache, pkgs []cache.Package) error {
+// Download makes a best-effort attempt at downloading previously cached build artifacts.
+// Returns detailed results for each package to enable smarter retry decisions.
+func (rs *GSUtilCache) Download(ctx context.Context, dst cache.LocalCache, pkgs []cache.Package) map[string]cache.DownloadResult {
+	results := make(map[string]cache.DownloadResult)
 	fmt.Printf("☁️  downloading %d cached build artifacts\n", len(pkgs))
+
 	var (
 		files []string
 		dest  string
@@ -108,17 +111,21 @@ func (rs *GSUtilCache) Download(ctx context.Context, dst cache.LocalCache, pkgs
 	for _, pkg := range pkgs {
 		fn, exists := dst.Location(pkg)
 		if exists {
+			results[pkg.FullName()] = cache.DownloadResult{Status: cache.DownloadStatusSkipped}
 			continue
 		}
 		version, err := pkg.Version()
 		if err != nil {
 			log.WithError(err).WithField("package", pkg.FullName()).Warn("Failed to get version for package, skipping")
+			results[pkg.FullName()] = cache.DownloadResult{Status: cache.DownloadStatusFailed, Err: err}
 			continue
 		}
 		if dest == "" {
 			dest = filepath.Dir(fn)
 		} else if dest != filepath.Dir(fn) {
-			return fmt.Errorf("gsutil only supports one target folder, not %s and %s", dest, filepath.Dir(fn))
+			err := fmt.Errorf("gsutil only supports one target folder, not %s and %s", dest, filepath.Dir(fn))
+			results[pkg.FullName()] = cache.DownloadResult{Status: cache.DownloadStatusFailed, Err: err}
+			continue
 		}
 
 		pair := urlPair{
@@ -129,7 +136,7 @@ func (rs *GSUtilCache) Download(ctx context.Context, dst cache.LocalCache, pkgs
 		urls = append(urls, pair.gzURL, pair.tarURL)
 	}
 	if len(urls) == 0 {
-		return nil
+		return results
 	}
 
 	args := append([]string{"stat"}, urls...)
@@ -142,21 +149,49 @@ func (rs *GSUtilCache) Download(ctx context.Context, dst cache.LocalCache, pkgs
 	err := cmd.Run()
 	if err != nil && (!strings.Contains(stderrBuffer.String(), "No URLs matched")) {
 		log.Debugf("gsutil stat returned non-zero exit code: [%v], stderr: [%v]", err, stderrBuffer.String())
-		return fmt.Errorf("failed to check if files exist in remote cache: %w", err)
+		// Mark all pending packages as failed
+		for pkg := range packageToURLMap {
+			if _, exists := results[pkg.FullName()]; !exists {
+				results[pkg.FullName()] = cache.DownloadResult{Status: cache.DownloadStatusFailed, Err: err}
+			}
+		}
+		return results
 	}
 
 	existingURLs := parseGSUtilStatOutput(strings.NewReader(stdoutBuffer.String()))
-	for _, urls := range packageToURLMap {
+	packagesToDownload := make(map[cache.Package]bool)
+	for pkg, urls := range packageToURLMap {
 		if _, exists := existingURLs[urls.gzURL]; exists {
 			files = append(files, urls.gzURL)
+			packagesToDownload[pkg] = true
 			continue
 		}
 		if _, exists := existingURLs[urls.tarURL]; exists {
 			files = append(files, urls.tarURL)
+			packagesToDownload[pkg] = true
+			continue
+		}
+		// Package not found in remote cache
+		results[pkg.FullName()] = cache.DownloadResult{Status: cache.DownloadStatusNotFound}
+	}
+
+	if len(files) > 0 {
+		transferErr := gsutilTransfer(dest, files)
+		for pkg := range packagesToDownload {
+			if transferErr != nil {
+				results[pkg.FullName()] = cache.DownloadResult{Status: cache.DownloadStatusFailed, Err: transferErr}
+			} else {
+				// Verify the file was actually downloaded
+				if _, exists := dst.Location(pkg); exists {
+					results[pkg.FullName()] = cache.DownloadResult{Status: cache.DownloadStatusSuccess}
+				} else {
+					results[pkg.FullName()] = cache.DownloadResult{Status: cache.DownloadStatusFailed, Err: fmt.Errorf("file not found after transfer")}
+				}
+			}
 		}
 	}
 
-	return gsutilTransfer(dest, files)
+	return results
 }
 
 // Upload makes a best effort to upload the build artifacts to a remote cache
@@ -195,7 +230,7 @@ func (rs *GSUtilCache) UploadFile(ctx context.Context, filePath string, key stri
 // HasFile checks if a file exists in the remote cache with the given key
 func (rs *GSUtilCache) HasFile(ctx context.Context, key string) (bool, error) {
 	target := fmt.Sprintf("gs://%s/%s", rs.BucketName, key)
-	
+
 	cmd := exec.CommandContext(ctx, "gsutil", "stat", target)
 	if err := cmd.Run(); err != nil {
 		// gsutil stat returns non-zero exit code if file doesn't exist
@@ -209,7 +244,7 @@ func (rs *GSUtilCache) HasFile(ctx context.Context, key string) (bool, error) {
 		}
 		return false, fmt.Errorf("failed to check if file exists at %s: %w", target, err)
 	}
-	
+
 	return true, nil
 }
 

pkg/leeway/cache/remote/no_cache.go

@@ -19,9 +19,14 @@ func (NoRemoteCache) ExistingPackages(ctx context.Context, pkgs []cache.Package)
 	return map[cache.Package]struct{}{}, nil
 }
 
-// Download makes a best-effort attempt at downloading previously cached build artifacts
-func (NoRemoteCache) Download(ctx context.Context, dst cache.LocalCache, pkgs []cache.Package) error {
-	return nil
+// Download makes a best-effort attempt at downloading previously cached build artifacts.
+// NoRemoteCache always returns NotFound for all packages since there is no remote cache.
+func (NoRemoteCache) Download(ctx context.Context, dst cache.LocalCache, pkgs []cache.Package) map[string]cache.DownloadResult {
+	results := make(map[string]cache.DownloadResult)
+	for _, pkg := range pkgs {
+		results[pkg.FullName()] = cache.DownloadResult{Status: cache.DownloadStatusNotFound}
+	}
+	return results
 }
 
 // Upload makes a best effort to upload the build artifacts to a remote cache

pkg/leeway/cache/remote/no_cache_test.go

@@ -86,9 +86,17 @@ func TestDownload(t *testing.T) {
 		pkgs = append(pkgs, packages[i])
 	}
 
-	err := noCache.Download(context.Background(), localCache, pkgs)
-	if err != nil {
-		t.Errorf("Download() error = %v", err)
+	results := noCache.Download(context.Background(), localCache, pkgs)
+	// NoRemoteCache should return NotFound for all packages
+	for _, pkg := range pkgs {
+		result, ok := results[pkg.FullName()]
+		if !ok {
+			t.Errorf("Download() missing result for package %s", pkg.FullName())
+			continue
+		}
+		if result.Status != cache.DownloadStatusNotFound {
+			t.Errorf("Download() status = %v, want %v", result.Status, cache.DownloadStatusNotFound)
+		}
 	}
 }