fix: handle non-zipped artifact downloads#13013
fix: handle non-zipped artifact downloads#13013dsanders11 wants to merge 2 commits intocli:trunkfrom
Conversation
|
Thanks for your pull request! Unfortunately, it doesn't meet the minimum requirements for review:
Please update your PR to address the above. Requirements:
This PR will be automatically closed in 7 days if these requirements are not met. |
There was a problem hiding this comment.
Pull request overview
This PR updates gh run download to support downloading GitHub Actions artifacts that are not delivered as zip archives (e.g., uploaded with archive: false), avoiding the current unconditional unzip behavior that fails on non-zip payloads.
Changes:
- Extend the download platform interface to pass artifact name into the download implementation.
- Add a non-zip download path in the HTTP downloader based on the response
Content-Type. - Add test coverage and a fixture for non-zipped artifact downloads.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/cmd/run/download/http.go | Adds non-zip handling and updates downloader signature to accept artifact name |
| pkg/cmd/run/download/http_test.go | Updates zip test to set Content-Type and adds a non-zip download test |
| pkg/cmd/run/download/fixtures/mytest.json | Adds a fixture representing non-zipped artifact content |
| pkg/cmd/run/download/download.go | Updates platform interface usage to pass artifact name to Download |
| pkg/cmd/run/download/download_test.go | Updates fake platform to match the new Download signature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if resp.Header.Get("Content-Type") != "application/zip" { | ||
| destPath, err := destDir.Join(name) |
There was a problem hiding this comment.
The zip-vs-nonzip decision is based on a strict Content-Type == "application/zip" check. In practice servers often include parameters (e.g. application/zip; charset=binary), omit the header, or use application/octet-stream for zip content, which would route zip downloads into the raw-file path. Parse the media type (e.g. via mime.ParseMediaType) and/or fall back to detecting/attempting zip extraction (and on zip: not a valid zip file, retry as a raw download).
There was a problem hiding this comment.
Considering these are GitHub controlled servers, I think it's reasonable to just use the Content-Type header to make the distinction.
Fixes #13012.