diff --git a/pkg/cli/logs_download.go b/pkg/cli/logs_download.go index 1a23f4f5e2..7b5daf3564 100644 --- a/pkg/cli/logs_download.go +++ b/pkg/cli/logs_download.go @@ -477,7 +477,16 @@ func listArtifacts(outputDir string) ([]string, error) { // failing the entire download. func isNonZipArtifactError(output []byte) bool { s := string(output) - return strings.Contains(s, "zip: not a valid zip file") || strings.Contains(s, "error extracting zip archive") + return strings.Contains(s, "zip: not a valid zip file") +} + +// isCaseCollisionArtifactError reports whether gh run download failed because +// a zip extraction attempted to write a file that already exists. This can +// happen on case-insensitive filesystems (e.g. macOS) when an artifact +// contains files whose names differ only by case. +func isCaseCollisionArtifactError(output []byte) bool { + s := string(output) + return strings.Contains(s, "error extracting zip archive") && strings.Contains(s, "file exists") } // isDockerBuildArtifact reports whether an artifact name represents a .dockerbuild artifact. @@ -754,6 +763,9 @@ func downloadRunArtifacts(ctx context.Context, runID int64, outputDir string, ve // skippedNonZipArtifacts is set when gh run download fails due to non-zip artifacts // that were not detected during the listing phase (e.g., listing failed). var skippedNonZipArtifacts bool + // skippedCaseCollisionArtifacts is set when gh run download fails because a single + // artifact contains case-colliding paths on case-insensitive filesystems. + var skippedCaseCollisionArtifacts bool if err != nil { // Stop spinner on error @@ -799,6 +811,9 @@ func downloadRunArtifacts(ctx context.Context, runID int64, outputDir string, ve } fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Some artifacts could not be extracted (not a valid zip archive) and were skipped: "+msg)) skippedNonZipArtifacts = true + } else if isCaseCollisionArtifactError(output) { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Some artifacts could not be fully extracted due to case-colliding file paths. Retrying artifacts individually and continuing.")) + skippedCaseCollisionArtifacts = true } else { return fmt.Errorf("failed to download artifacts for run %d: %w (output: %s)", runID, err, string(output)) } @@ -811,6 +826,33 @@ func downloadRunArtifacts(ctx context.Context, runID int64, outputDir string, ve retryCriticalArtifacts(ctx, runID, outputDir, verbose, owner, repo, hostname, artifactFilter) } + // When bulk download fails on case-colliding entries, gh CLI aborts and may skip + // artifacts that appear later in the run. Retry artifacts individually so one bad + // artifact does not block the rest. + if skippedCaseCollisionArtifacts { + retryNames := downloadableNames + if len(retryNames) == 0 { + // Initial artifact listing was unavailable, so fetch names now for targeted retry. + artifactNamesRetry, retryListErr := listRunArtifactNames(ctx, runID, owner, repo, hostname, verbose) + if retryListErr != nil { + return fmt.Errorf("bulk artifact download hit case-colliding entries and could not list artifacts for individual retry: %w", retryListErr) + } + for _, name := range artifactNamesRetry { + if isDockerBuildArtifact(name) { + continue + } + if artifactMatchesFilter(name, artifactFilter) { + retryNames = append(retryNames, name) + } + } + } + if len(retryNames) > 0 { + if err := downloadArtifactsByName(ctx, runID, outputDir, retryNames, verbose, owner, repo, hostname); err != nil { + return err + } + } + } + if skippedNonZipArtifacts && fileutil.IsDirEmpty(outputDir) { // All artifacts were non-zip (none could be extracted) so nothing was downloaded. // Treat this the same as a run with no artifacts — the audit will rely solely on diff --git a/pkg/cli/logs_download_test.go b/pkg/cli/logs_download_test.go index 3ef7a04693..28b50f8558 100644 --- a/pkg/cli/logs_download_test.go +++ b/pkg/cli/logs_download_test.go @@ -130,9 +130,9 @@ func TestIsNonZipArtifactError(t *testing.T) { expected: true, }, { - name: "error extracting zip archive", + name: "generic extraction error without zip signature", output: "error downloading some-artifact: error extracting zip archive: unexpected EOF", - expected: true, + expected: false, }, { name: "both patterns present", @@ -166,6 +166,39 @@ func TestIsNonZipArtifactError(t *testing.T) { } } +func TestIsCaseCollisionArtifactError(t *testing.T) { + tests := []struct { + name string + output string + expected bool + }{ + { + name: "case-collision file exists error", + output: "error downloading repo-memory-default: error extracting zip archive: error extracting \"memory.md\": open /tmp/artifacts/repo-memory-default/memory.md: file exists", + expected: true, + }, + { + name: "generic extraction error without file exists", + output: "error downloading some-artifact: error extracting zip archive: unexpected EOF", + expected: false, + }, + { + name: "file exists without extraction context", + output: "open /tmp/out/file.txt: file exists", + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := isCaseCollisionArtifactError([]byte(tt.output)) + if result != tt.expected { + t.Errorf("isCaseCollisionArtifactError(%q) = %v, want %v", tt.output, result, tt.expected) + } + }) + } +} + func TestIsDockerBuildArtifact(t *testing.T) { tests := []struct { name string