ci: keep ineligible runs out of the review concurrency group - #2348
Merged
Conversation
kriszyp review finding on the pin-bump series: workflow-level cancel-in-progress fires before job if:, so in opt-in mode an ineligible event (a push, or a ready-flip without the opt-in label) cancels an in-flight label-triggered review and then skips — silently losing the requested review. Ineligible runs now take a unique run_id group and can never cancel an eligible one; eligible runs keep cancelling each other (the debounce contract). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S94XethbGXpAb4DRKMD4kt
Contributor
|
Note Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported. |
github-actions
Bot
requested review from
Ethan-Arrowood,
cb1kenobi and
kriszyp
August 26, 2026 16:26
heskew
commented
Aug 26, 2026
Review finding (Codex + kriszyp, independently): the eligibility predicate admitted every labeled event, so in opt-in mode an unrelated label applied mid-review joined the eligible concurrency group, cancelled the running review, and the replacement then failed the reusable's exact-label gate — no completed review. The same unrelated-label cancellation existed in always-on mode before this series (any labeled event shared the group and authorize then skipped). The labeled branch now requires the provider's own label, in both the concurrency predicate and the review job gate, and unrelated-label events are ineligible in both modes. Event matrix (opt-in / always-on): - labeled(provider label): eligible / eligible — supersedes in-flight - labeled(other): ineligible / ineligible (was: cancelled + no review) - synchronize: ineligible / eligible - ready_for_review + label: eligible / eligible - ready_for_review, no label: ineligible / eligible - opened, reopened: ineligible / eligible Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S94XethbGXpAb4DRKMD4kt
kriszyp
reviewed
Aug 26, 2026
kriszyp
left a comment
kriszyp
left a comment
Member
There was a problem hiding this comment.
Still not sure what this does, but ok.
🤖 Reviewed with Codex
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
What this changes, concretely
Both review callers use a shared concurrency group per PR with
cancel-in-progress: true, so a newer run can cancel an in-flight review. GitHub evaluates that cancellation when the new run is queued — before any jobif:runs. That means an event that was never going to produce a review could still kill one in progress:bug) mid-review did the same — the replacement died at the reusable's exact-label authorize gate.The fix
The concurrency group key now ends in
eligibleonly for events that would actually produce a review; every other event gets a uniquegithub.run_idsuffix, i.e. its own group, so it can never cancel anything. "Would produce a review" means, per caller:labeled+label.name == 'claude-review'/'gemini-review'), or*_ALWAYS_ONis on, orThe review job's
if:uses the identical predicate (kept textually in sync — Actions YAML has no anchors), so eligible runs both start the review and may supersede an in-flight one, and ineligible runs do neither.Findings by @kriszyp and the Codex pass on the pin-bump series (harper-pro#766, #2330/#2331); the unrelated-label variant predated this series in always-on mode and is closed by the same change. Full event matrix in the fa04115 commit message.
Merge order: this before #2330/#2331 — the sync PRs carry this fix and must stay byte-identical to main.
🤖 Generated with Claude Code
https://claude.ai/code/session_01S94XethbGXpAb4DRKMD4kt