Repository navigation
ci: cancel PR workflows on close - #1723
Conversation
Signed-off-by: key4ng <rukeyang@gmail.com>
|
Note Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a GitHub Actions workflow that runs when a pull request is closed, computes PR-specific concurrency groups from a matrix of workflow templates, and cancels matching in-progress runs while logging the PR number and resolved group. ChangesCI workflow cancellation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| include: | ||
| # Keep these in sync with the concurrency groups used by PR workflows. | ||
| - workflow: PR Test (SMG) | ||
| group: gateway-tests-pr-{0} | ||
| - workflow: PR Test (MLX) | ||
| group: mlx-tests-pr-{0} | ||
| - workflow: PR Validation | ||
| group: pr-validation-{0} | ||
| - workflow: PR Labeler | ||
| group: labeler-{0} | ||
| - workflow: Claude PR Review | ||
| group: claude-review-{0} | ||
| - workflow: Benchmark - Tokenizer | ||
| group: benchmark-tokenizer-refs/pull/{0}/merge |
There was a problem hiding this comment.
🟡 Nit: claude-code-review.yml has a second PR concurrency group — claude-respond-${{ github.event.issue.number || github.event.pull_request.number }} (line 307) — that isn't covered here. Its original cancel-in-progress: false prevents new comment triggers from cancelling an in-flight response, but when the PR is closed there's no audience for the response, so cancelling it would save runner minutes.
If intentionally excluded, a comment in the matrix noting why would help future maintainers keeping this list in sync.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08dd08eb47
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| group: ${{ format(matrix.group, github.event.pull_request.number) }} | ||
| cancel-in-progress: true |
There was a problem hiding this comment.
When a PR is closed and then quickly reopened, the pull_request.closed run can still be queued after the reopened workflows have started; because this job unconditionally enters the same PR concurrency group with cancel-in-progress: true, it can cancel the fresh reopened CI (for example, .github/workflows/pr-test-rust.yml triggers on reopened and uses gateway-tests-pr-{number}). Please avoid touching the target cancellation groups unless the PR is still closed/current, or use an API-based cancellation path that checks current PR state before canceling.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Looks good. The workflow correctly targets all PR concurrency groups, permissions are minimal, and fail-fast: false is appropriate.
Two existing comments remain relevant:
- 🟡 Nit (line 30): The
claude-respondconcurrency group fromclaude-code-review.ymlisn't covered — worth adding to save runner time on closed PRs. - 🟡 Nit (line 41, flagged by another reviewer): Close/reopen race — if the closed-event run is delayed past a reopen, it could cancel the freshly-triggered CI. Low probability but worth noting.
No 🔴 Important issues found. 0 important · 2 nits (pre-existing)
1 participant
Description
Problem
Long-running PR workflows can keep consuming runners after a PR is merged or manually closed because the existing PR workflows only run on open/update/reopen activity. No final close-event run is created to cancel the active PR concurrency groups.
Solution
Add a lightweight
pull_request.closedworkflow that starts no-op matrix jobs in the same concurrency groups as the existing PR workflows. Withcancel-in-progress: true, GitHub Actions cancels any active run in those groups when the PR closes. This keeps the change scoped to workflows that already define PR concurrency groups and does not modify release workflows.Changes
.github/workflows/cancel-pr-workflows.yml.Test Plan
actionlint .github/workflows/cancel-pr-workflows.ymlpre-commit run --files .github/workflows/cancel-pr-workflows.ymlgit diff --checkChecklist
cargo +nightly fmtpassescargo clippy --all-targets --all-features -- -D warningspassesSummary by CodeRabbit