ci: upgrade upload-artifac from v5 to v6#54
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughBumped multiple GitHub Actions artifact upload/download steps from Changes
Sequence Diagram(s)sequenceDiagram
participant PR as Pull Request (closed)
participant Script as github-script Job
participant API as GitHub REST API
participant WF as Target Workflow Runs
PR->>Script: trigger on closed event (provides PR number & head.sha)
Script->>API: list workflow runs for `pr-build.yml` and `system-test.yml` (states: queued,in_progress)
API-->>Script: returns workflow runs
loop each run where run.pull_request.number == PR.number AND run.head_sha == PR.head.sha
Script->>API: cancel workflow run
API-->>Script: cancellation confirmation
Script->>WF: log cancelled run id and status
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
There was a problem hiding this comment.
1 issue found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/coverage-update-baseline.yml">
<violation number="1" location=".github/workflows/coverage-update-baseline.yml:49">
P2: `upload-artifact` is upgraded to v6 but `download-artifact` in the same workflow (line 76) remains at v5. These paired actions should be upgraded together to maintain version consistency. Both v6 releases are coordinated updates to Node.js 24 runtime.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/coverage-update-baseline.yml (1)
75-75: Consider upgradingdownload-artifactto v6 for consistency.While this PR upgrades
upload-artifactto v6 (line 49), the correspondingdownload-artifactaction remains at v5. For consistency and to avoid potential compatibility issues between upload and download actions, consider upgradingdownload-artifactto v6 as well.♻️ Suggested upgrade for consistency
- uses: actions/download-artifact@v5 + uses: actions/download-artifact@v6🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/coverage-update-baseline.yml at line 75, Replace the hardcoded uses reference for the download-artifact action to match the upload-artifact upgrade: change the string "actions/download-artifact@v5" to "actions/download-artifact@v6" so both upload and download artifact steps use v6; ensure the updated uses entry for "actions/download-artifact" is applied in the workflow file alongside the already-upgraded "actions/upload-artifact".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/coverage-gen.yml:
- Line 47: The workflow currently uses actions/upload-artifact@v6 which has
breaking changes (Node.js 24 runtime and self-hosted runner ≥2.327.1); either
pin to actions/upload-artifact@v5 or explicitly ensure compatibility by (a)
setting the runner/node toolchain to Node.js 24 in the job (e.g., via
actions/setup-node) if you adopt `@v6`, and (b) verifying any self-hosted runners
meet GitHub Actions Runner ≥2.327.1; update the uses line
(actions/upload-artifact@v6) accordingly and document the chosen approach.
---
Nitpick comments:
In @.github/workflows/coverage-update-baseline.yml:
- Line 75: Replace the hardcoded uses reference for the download-artifact action
to match the upload-artifact upgrade: change the string
"actions/download-artifact@v5" to "actions/download-artifact@v6" so both upload
and download artifact steps use v6; ensure the updated uses entry for
"actions/download-artifact" is applied in the workflow file alongside the
already-upgraded "actions/upload-artifact".
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5b667e75-038c-4d0a-a4fc-738496f7ea10
📒 Files selected for processing (5)
.github/workflows/coverage-gen.yml.github/workflows/coverage-update-baseline.yml.github/workflows/math-check.yml.github/workflows/pr-check.yml.github/workflows/system-test.yml
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/pr-cancel.yml">
<violation number="1" location=".github/workflows/pr-cancel.yml:33">
P1: Wrap `cancelWorkflowRun` in a try/catch. A race condition between listing and cancelling (e.g., run completes in the interim) causes a 409 error that aborts the script, skipping cancellation of any remaining runs.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| for (const run of data.workflow_runs) { | ||
| if (run.head_sha === headSha) { | ||
| await github.rest.actions.cancelWorkflowRun({ |
There was a problem hiding this comment.
P1: Wrap cancelWorkflowRun in a try/catch. A race condition between listing and cancelling (e.g., run completes in the interim) causes a 409 error that aborts the script, skipping cancellation of any remaining runs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/pr-cancel.yml, line 33:
<comment>Wrap `cancelWorkflowRun` in a try/catch. A race condition between listing and cancelling (e.g., run completes in the interim) causes a 409 error that aborts the script, skipping cancellation of any remaining runs.</comment>
<file context>
@@ -0,0 +1,42 @@
+
+ for (const run of data.workflow_runs) {
+ if (run.head_sha === headSha) {
+ await github.rest.actions.cancelWorkflowRun({
+ owner: context.repo.owner,
+ repo: context.repo.repo,
</file context>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/pr-cancel.yml:
- Around line 24-33: listWorkflowRuns is currently called without pagination and
filters only by head_sha, which can miss runs beyond the first page and
accidentally cancel runs from non-PR events; change to fetch all pages (use
octokit.paginate or loop with per_page) for github.rest.actions.listWorkflowRuns
for the given workflowId/status, then filter the aggregated data.workflow_runs
to runs where run.head_sha === headSha AND run.event === 'pull_request' AND
run.pull_requests.some(pr => pr.number === prNumber) (use the PR number variable
in scope), and only then call github.rest.actions.cancelWorkflowRun for each
matched run.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 16b1a786-a13d-482c-93db-b9e402a8a749
📒 Files selected for processing (1)
.github/workflows/pr-cancel.yml
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/pr-cancel.yml">
<violation number="1" location=".github/workflows/pr-cancel.yml:39">
P1: The `run.pull_requests` field in the GitHub API is frequently empty — especially for fork PRs, but also in other cases (merged PRs, `pull_request_target` triggers). When it's empty, `isTargetPr` evaluates to `false`/`undefined`, and the `&&` condition silently skips runs that should be cancelled.
Consider falling back to SHA-only matching when `pull_requests` is unavailable, so the cancel logic remains reliable regardless of whether GitHub populates the field.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| ); | ||
|
|
||
| for (const run of runs) { | ||
| const isTargetPr = run.pull_requests?.some((pr) => pr.number === prNumber); |
There was a problem hiding this comment.
P1: The run.pull_requests field in the GitHub API is frequently empty — especially for fork PRs, but also in other cases (merged PRs, pull_request_target triggers). When it's empty, isTargetPr evaluates to false/undefined, and the && condition silently skips runs that should be cancelled.
Consider falling back to SHA-only matching when pull_requests is unavailable, so the cancel logic remains reliable regardless of whether GitHub populates the field.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/pr-cancel.yml, line 39:
<comment>The `run.pull_requests` field in the GitHub API is frequently empty — especially for fork PRs, but also in other cases (merged PRs, `pull_request_target` triggers). When it's empty, `isTargetPr` evaluates to `false`/`undefined`, and the `&&` condition silently skips runs that should be cancelled.
Consider falling back to SHA-only matching when `pull_requests` is unavailable, so the cancel logic remains reliable regardless of whether GitHub populates the field.</comment>
<file context>
@@ -18,18 +18,26 @@ jobs:
- for (const run of data.workflow_runs) {
- if (run.head_sha === headSha) {
+ for (const run of runs) {
+ const isTargetPr = run.pull_requests?.some((pr) => pr.number === prNumber);
+ if (run.head_sha === headSha && isTargetPr) {
await github.rest.actions.cancelWorkflowRun({
</file context>
| const isTargetPr = run.pull_requests?.some((pr) => pr.number === prNumber); | |
| const isTargetPr = !run.pull_requests?.length || run.pull_requests.some((pr) => pr.number === prNumber); |
What does this PR do?
Why are these changes required?
This PR has been tested by:
Follow up
Extra details
Summary by cubic
Upgrade
actions/upload-artifact/actions/download-artifactto v6 across CI and switchsystem-testconcurrency to PR numbers for safer parallel runs. Add a PR-close workflow that cancels only the matching PR runs to keep CI clean.New Features
pr-cancel.ymlto cancelpr-build.ymlandsystem-test.ymlon PR close viaactions/github-script@v8; filters by PR number and head SHA and targets in-progress/queued runs.Dependencies
actions/upload-artifact@v6andactions/download-artifact@v6in coverage (gen, upload, update-baseline), math-check, pr-check, and system-test workflows.Written for commit 7735ebf. Summary will update on new commits.
Summary by CodeRabbit