Skip to content

Stop late draft runs cancelling real PR CI - #1708

Open
Mikola Lysenko (mikolalysenko) wants to merge 1 commit into
mainfrom
ci-janitor/draft-run-cancels-ready
Open

Mikola Lysenko (mikolalysenko) wants to merge 1 commit into
mainfrom
ci-janitor/draft-run-cancels-ready

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 24, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

A PR's ci-ok can go green on a head where no CI job ran, and the real run gets cancelled. It happened twice in the last ~3 hours:

PR head real run (ready_for_review) late draft run result
#1706 84d95a4 (committed 03:14:17Z, ready 03:14:22Z) 40057483229 attempt 1 cancelled 40057484023: every job skipped, ci-ok success needed manual re-run (attempt 2)
#1707 507fbf7 (committed 05:08:20Z, ready 05:08:25Z) 40065091770 attempt 1 cancelled 40065092123: every job skipped, ci-ok success in 14s needed manual re-run (attempt 2)

Merges are still safe (the merge queue runs full CI), but a false green feeds the final-review routine, which can enqueue an untested PR and get it evicted from the queue, and every occurrence costs a manual re-run.

Root cause

Agents push and run gh pr ready a few seconds later. GitHub can deliver that push's synchronize event, whose payload still says draft: true, after ready_for_review. Both runs shared ci-<PR number> with cancel-in-progress: true, so the late draft run cancelled the real run, then skipped every job on the github.event.pull_request.draft != true gates, and ci-ok reported success.

Fix

Draft PR runs get their own concurrency group (ci-<PR>-draft). A draft-payload run can't cancel a ready-state run any more. Draft pushes still supersede each other, and so do ready pushes. Push, merge_group and schedule groups don't change (github.event.pull_request is null there). Required check names and the ci-ok gate don't change.

A regression test in scripts/tests/test_ci_scheduling.py pins the split.

Proof

  • New test fails against the old ci.yml and passes with the fix. The full python3 -B -m unittest discover -s scripts/tests passes (373 tests, 6 skipped).
  • actionlint (37 pre-existing findings, YAML-alias related) and zizmor --offline (59 findings) give the same results before and after.
  • Note: workflow changes only take effect on a new head commit.

Tests removed or moved

None.


Generated by Claude Code


Note

Low Risk
Workflow-only concurrency grouping change with a pinned unit test; no job logic, required check names, or ci-ok gates change.

Overview
Fixes a race where a late synchronize event (payload still draft: true) could land after ready_for_review, share the same PR concurrency group, cancel the real CI run, skip all draft-gated jobs, and leave ci-ok green without testing the head.

CI workflow (ci.yml): appends ${{ github.event.pull_request.draft && '-draft' || '' }} to the workflow concurrency.group, so draft and ready runs no longer cancel each other; push, merge-group, and nightly groups are unchanged. Comments document the failure mode.

Regression test (test_ci_scheduling.py): test_late_draft_run_cannot_cancel_the_ready_run asserts the concurrency group expression includes the draft suffix.

Reviewed by Cursor Bugbot for commit 69910d1. Configure here.


Generated by Claude Code

An agent pushes a commit and runs `gh pr ready` seconds later. GitHub
can deliver that push's `synchronize` event (payload still draft: true)
after `ready_for_review`. Both runs shared one concurrency group with
cancel-in-progress, so the late draft run cancelled the real run,
skipped every job on the draft gate, and reported ci-ok green on a head
nothing had tested. Seen on #1706 and #1707 today; both needed a manual
re-run to get a real verdict.

Give draft PR runs their own group so they can never cancel a
ready-state run. Draft pushes still supersede each other, as do ready
pushes. Add a scheduling test that pins the split.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01EtzNTGaPfJK39cMX68F1qw
@mikolalysenko Mikola Lysenko (mikolalysenko) added the ci-janitor Opened by the CI janitor routine (flakes, redundant tests, CI perf) label Oct 24, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

bugbot run


Generated by Claude Code

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 69910d1. Configure here.

@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 24, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Labeled Ready for review by the burn-down agent.

  • Head: 69910d19ef105c10076a4979604026a7233480ac
  • CI: 38/38 non-skipped checks green, no main-wide failures
  • Bugbot: reviewed this head, no findings; no unresolved review threads
  • Mergeable, no CHANGELOG.md changes

Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Final review brief

What it does. When an agent pushes and then immediately runs gh pr ready, the push's synchronize event (whose payload still says draft) can arrive after ready_for_review. Both runs shared one concurrency group, so the late draft run cancelled the real one. Its jobs were all skipped, which left ci-ok green on a head nothing had tested. Draft-payload runs now get their own -draft concurrency group, so they can't cancel the real run.

Risk: low. One expression in ci.yml's concurrency group, plus a regression test. For non-PR events github.event.pull_request.draft is null, so push, schedule and merge_group groups are unchanged.

Look here

Verified

  • Read the diff (2 files) and ran python3 -m unittest scripts/tests/test_ci_scheduling.py locally: 10 tests OK.
  • Checked that the suffix doesn't affect push, nightly or merge_group groups.
  • CHANGELOG.md is untouched.
  • CI: ci-ok and clippy green on 69910d19. Cursor Bugbot passed with no open threads.

Changes I made: none.

Open questions (not blocking):

  • A PR converted back to draft and pushed no longer cancels its in-flight ready run. That wastes some CI time but is harmless.
  • The late draft run still posts its own (all-skipped) green ci-ok on the same head. It finishes in about a minute, while the real ci-ok waits on every job, so the real verdict should land last. Worth a glance if you know of a case where the real run's ci-ok can finish first.

Auto-merge is armed, so approving sends this straight to the merge queue.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-janitor Opened by the CI janitor routine (flakes, redundant tests, CI perf) Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants