Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 43 additions & 1 deletion pkg/cli/logs_download.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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))
}
Expand All @@ -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
Expand Down
37 changes: 35 additions & 2 deletions pkg/cli/logs_download_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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
Expand Down