ci: exclude ready_for_review from the always-on cancelling arm - #2357
Merged
Conversation
Codex follow-up on #2353: ready_for_review is a non-labeled event, so the ALWAYS_ON arm of the concurrency predicate still placed it in the shared cancelling group in always-on mode — a label-opted bot PR's ready-flip could cancel the labeler's in-flight review and then fail author-based authorization. The cancelling set now excludes ready_for_review in BOTH modes; the job gates are unchanged (ready-flips run review, in their own run_id group). 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 23:54
heskew
added a commit
that referenced
this pull request
Aug 26, 2026
…2357) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S94XethbGXpAb4DRKMD4kt
heskew
enabled auto-merge (squash)
August 27, 2026 00:11
heskew
disabled auto-merge
August 27, 2026 00:11
heskew
added a commit
that referenced
this pull request
Aug 27, 2026
* ci(v5.1): sync bot caller workflows with main Content matches bump-ai-review-prompts-pin (#2328) — identical to main once that merges. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S94XethbGXpAb4DRKMD4kt * ci: adopt ai-review-prompts #90 cost gates (pin be549ad) + ready_for_review trigger Draft PRs skip review until flipped ready (label still opts one in), mechanical diffs skip pre-run, reasoning effort scales with diff size (60/high, 1500/xhigh, else max), synchronize runs debounce 120s. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S94XethbGXpAb4DRKMD4kt * ci: admit ready_for_review on label-opted PRs in opt-in mode kriszyp review finding on the pin-bump PR: with *_ALWAYS_ON unset, the caller gate admitted only labeled events, so a PR opted in by label while draft never resumed review when flipped ready — the event died at the caller gate. Admit ready_for_review when the opt-in label is still present. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S94XethbGXpAb4DRKMD4kt * ci: keep ineligible runs out of the review concurrency group kriszyp review finding: 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 * ci: only the provider's own label makes a labeled run eligible 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 * ci: ready_for_review runs review but never cancels kriszyp follow-up finding: the label-opted ready_for_review event sat in the shared eligible concurrency group, so on a bot-authored PR it could cancel the trusted labeler's in-flight review and then be rejected by author-based authorization — no completed review. The cancelling set (concurrency predicate) now excludes ready_for_review; the running set (job gate) keeps it, so trusted-author ready-flips still review, without the power to cancel. Revisit if the reusable gains persisted-label authorization. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S94XethbGXpAb4DRKMD4kt * ci: exclude ready_for_review from the always-on cancelling arm (see #2357) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S94XethbGXpAb4DRKMD4kt --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
heskew
added a commit
that referenced
this pull request
Aug 27, 2026
* ci(v5.2): sync bot caller workflows with main Content matches bump-ai-review-prompts-pin (#2328) — identical to main once that merges. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S94XethbGXpAb4DRKMD4kt * ci: adopt ai-review-prompts #90 cost gates (pin be549ad) + ready_for_review trigger Draft PRs skip review until flipped ready (label still opts one in), mechanical diffs skip pre-run, reasoning effort scales with diff size (60/high, 1500/xhigh, else max), synchronize runs debounce 120s. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S94XethbGXpAb4DRKMD4kt * ci: admit ready_for_review on label-opted PRs in opt-in mode kriszyp review finding on the pin-bump PR: with *_ALWAYS_ON unset, the caller gate admitted only labeled events, so a PR opted in by label while draft never resumed review when flipped ready — the event died at the caller gate. Admit ready_for_review when the opt-in label is still present. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S94XethbGXpAb4DRKMD4kt * ci: keep ineligible runs out of the review concurrency group kriszyp review finding: 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 * ci: only the provider's own label makes a labeled run eligible 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 * ci: ready_for_review runs review but never cancels kriszyp follow-up finding: the label-opted ready_for_review event sat in the shared eligible concurrency group, so on a bot-authored PR it could cancel the trusted labeler's in-flight review and then be rejected by author-based authorization — no completed review. The cancelling set (concurrency predicate) now excludes ready_for_review; the running set (job gate) keeps it, so trusted-author ready-flips still review, without the power to cancel. Revisit if the reusable gains persisted-label authorization. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S94XethbGXpAb4DRKMD4kt * ci(v5.2): sync the always-on ready_for_review cancelling fix (see #2357) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S94XethbGXpAb4DRKMD4kt --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
Corrective follow-up to #2353, from the Codex pass:
ready_for_reviewis a non-labeled event, so theALWAYS_ONarm of the concurrency predicate still placed it in the shared cancelling group in always-on mode — a label-opted bot PR's ready-flip could cancel the labeler's in-flight review and then fail author-based authorization. The cancelling set now excludesready_for_reviewin both modes; the job gates are unchanged (ready-flips run review, in their ownrun_idgroup, so they run but never cancel).Propagation: #2354 (v5.1) carries this; a v5.2 sync PR follows this merge; harper-pro #766/#767/#768 updated. An event-matrix regression check (ready_for_review + ALWAYS_ON ⇒ runs-but-does-not-cancel) lands in ai-review-prompts alongside the examples.
Merge order: this, then #2354 and the v5.2 sync.
🤖 Generated with Claude Code
https://claude.ai/code/session_01S94XethbGXpAb4DRKMD4kt