P0: Release republish same tag (clobber existing assets safely) #31
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's GuideAdds a pre-release step in the GitHub Actions release workflow to safely delete any existing assets on a release with the same tag prior to creating a new release, while skipping deletion if no prior release exists. Sequence diagram for deleting existing release assets before republishsequenceDiagram
participant GA as GitHubActions_release_workflow
participant PS as PowerShell_step_Delete_existing_assets
participant GH as GitHub_REST_API
GA->>PS: Start delete existing release assets step
PS->>PS: Extract tag from GITHUB_REF
PS->>GH: GET /repos/{repo}/releases/tags/{tag}
alt Release for tag exists
GH-->>PS: 200 OK with release (includes assets)
PS->>PS: Check release object
alt Release has assets
loop For each asset in release.assets
PS->>GH: DELETE /repos/{repo}/releases/assets/{asset_id}
GH-->>PS: 204 No Content
end
else No assets in release
PS->>PS: Log No existing assets to delete
end
else Release for tag does not exist
GH-->>PS: 404 Not Found
PS->>PS: Log No existing release for tag
PS-->>GA: Exit step successfully (no deletion)
end
PS-->>GA: Deletion step complete
GA->>GA: Proceed to softprops/action-gh-release@v2 step
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The delete-assets step runs unconditionally for the job, whereas the release step is guarded with
if: startsWith(github.ref, 'refs/tags/'); consider adding the same condition to avoid invoking the GitHub Releases API for non-tag refs. - The 404 handling in the
Invoke-RestMethodcatch block assumes$_ .Exception.Response.StatusCodeis always present; adding a null check aroundResponse(or matching on message/content) would make this more robust to unexpected error shapes. - GitHub's REST API typically expects a
User-Agentheader; adding one to the$headersused byInvoke-RestMethodwill make the API calls more standards-compliant and easier to trace.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The delete-assets step runs unconditionally for the job, whereas the release step is guarded with `if: startsWith(github.ref, 'refs/tags/')`; consider adding the same condition to avoid invoking the GitHub Releases API for non-tag refs.
- The 404 handling in the `Invoke-RestMethod` catch block assumes `$_ .Exception.Response.StatusCode` is always present; adding a null check around `Response` (or matching on message/content) would make this more robust to unexpected error shapes.
- GitHub's REST API typically expects a `User-Agent` header; adding one to the `$headers` used by `Invoke-RestMethod` will make the API calls more standards-compliant and easier to trace.
## Individual Comments
### Comment 1
<location> `.github/workflows/release.yml:66-68` </location>
<code_context>
+ GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+ run: |
+ $tag = $env:GITHUB_REF -replace 'refs/tags/', ''
+ $headers = @{
+ Authorization = "Bearer $env:GITHUB_TOKEN"
+ Accept = "application/vnd.github+json"
+ }
+ try {
</code_context>
<issue_to_address>
**issue (bug_risk):** GitHub REST calls are missing a User-Agent header, which can cause API requests to be rejected.
Please add a required `User-Agent` header to `$headers` (for example, `User-Agent = "github-actions/$env:GITHUB_REPOSITORY"`) so these GitHub REST requests aren’t rejected or throttled intermittently.
</issue_to_address>
### Comment 2
<location> `.github/workflows/release.yml:72-78` </location>
<code_context>
+ }
+ try {
+ $release = Invoke-RestMethod -Method Get -Uri "https://api.github.com/repos/$env:GITHUB_REPOSITORY/releases/tags/$tag" -Headers $headers
+ } catch {
+ if ($_.Exception.Response.StatusCode.value__ -eq 404) {
+ Write-Host "No existing release for $tag"
+ exit 0
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The 404 handling assumes Response is non-null and may throw if the exception shape differs.
In some failure cases (e.g., network errors), `$_` may not have `.Exception.Response` or `.StatusCode`, causing this catch block to throw. Please add a null check (e.g. `if ($_.Exception.Response -and $_.Exception.Response.StatusCode.value__ -eq 404)`) or use null-propagation to ensure the 404 handling is safe for all error types.
```suggestion
} catch {
if ($_.Exception.Response -and $_.Exception.Response.StatusCode.value__ -eq 404) {
Write-Host "No existing release for $tag"
exit 0
}
throw
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+66
to
+68
| $headers = @{ | ||
| Authorization = "Bearer $env:GITHUB_TOKEN" | ||
| Accept = "application/vnd.github+json" |
There was a problem hiding this comment.
issue (bug_risk): GitHub REST calls are missing a User-Agent header, which can cause API requests to be rejected.
Please add a required User-Agent header to $headers (for example, User-Agent = "github-actions/$env:GITHUB_REPOSITORY") so these GitHub REST requests aren’t rejected or throttled intermittently.
Comment on lines
+72
to
+78
| } catch { | ||
| if ($_.Exception.Response.StatusCode.value__ -eq 404) { | ||
| Write-Host "No existing release for $tag" | ||
| exit 0 | ||
| } | ||
| throw | ||
| } |
There was a problem hiding this comment.
suggestion (bug_risk): The 404 handling assumes Response is non-null and may throw if the exception shape differs.
In some failure cases (e.g., network errors), $_ may not have .Exception.Response or .StatusCode, causing this catch block to throw. Please add a null check (e.g. if ($_.Exception.Response -and $_.Exception.Response.StatusCode.value__ -eq 404)) or use null-propagation to ensure the 404 handling is safe for all error types.
Suggested change
| } catch { | |
| if ($_.Exception.Response.StatusCode.value__ -eq 404) { | |
| Write-Host "No existing release for $tag" | |
| exit 0 | |
| } | |
| throw | |
| } | |
| } catch { | |
| if ($_.Exception.Response -and $_.Exception.Response.StatusCode.value__ -eq 404) { | |
| Write-Host "No existing release for $tag" | |
| exit 0 | |
| } | |
| throw | |
| } |
There was a problem hiding this comment.
Pull request overview
This PR implements functionality to safely republish releases with the same tag by deleting existing release assets before uploading new ones. This addresses issue #14 and enables clobber behavior for release assets.
Key Changes:
- Adds a new workflow step that queries the GitHub API to check for existing releases
- Deletes all assets from an existing release before the new release upload
- Gracefully handles cases where no prior release exists (404 responses)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+80
to
+84
| if (-not $release) { | ||
| Write-Host "No existing release data for $tag" | ||
| exit 0 | ||
| } | ||
There was a problem hiding this comment.
This null check is redundant. If the Invoke-RestMethod call on line 71 succeeds, it will always return a response object. The 404 case is already handled in the catch block above. This check can never be true after a successful API call, making these lines unreachable code.
Suggested change
| if (-not $release) { | |
| Write-Host "No existing release data for $tag" | |
| exit 0 | |
| } |
| foreach ($asset in $release.assets) { | ||
| Write-Host "Deleting asset $($asset.name)" | ||
| Invoke-RestMethod -Method Delete -Uri "https://api.github.com/repos/$env:GITHUB_REPOSITORY/releases/assets/$($asset.id)" -Headers $headers |
There was a problem hiding this comment.
The asset deletion loop lacks error handling. If deletion of any asset fails (e.g., due to network issues or API rate limits), the script will throw an exception and halt, potentially leaving the release in an inconsistent state with some assets deleted and others remaining. Consider adding try-catch blocks within the loop to handle individual asset deletion failures gracefully, or at minimum, add logging to identify which asset failed.
Suggested change
| Invoke-RestMethod -Method Delete -Uri "https://api.github.com/repos/$env:GITHUB_REPOSITORY/releases/assets/$($asset.id)" -Headers $headers | |
| try { | |
| Invoke-RestMethod -Method Delete -Uri "https://api.github.com/repos/$env:GITHUB_REPOSITORY/releases/assets/$($asset.id)" -Headers $headers | |
| } catch { | |
| Write-Warning "Failed to delete asset '$($asset.name)' (ID: $($asset.id)): $($_.Exception.Message)" | |
| } |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #14
Summary:
Tests:
Rollback:
Summary by Sourcery
CI: