Skip to content

build: stop running PR title check on synchronize - #203

Merged
tokoko merged 1 commit into
substrait-io:mainfrom
nielspardon:fix/pr-title-check-synchronize-race
Jul 14, 2026
Merged

build: stop running PR title check on synchronize#203
tokoko merged 1 commit into
substrait-io:mainfrom
nielspardon:fix/pr-title-check-synchronize-race

Conversation

@nielspardon

Copy link
Copy Markdown
Member

Problem

The PR Title Check registers as failing on some PRs even though the title conforms — see #198. The check ends up with two check-runs of the same name on the head commit, one SUCCESS and one CANCELLED, and GitHub's status rollup treats the cancelled one as not-passing.

Root cause

When Dependabot rebases a PR it force-pushes a new commit and updates the PR body in the same instant. That fires two pull_request_target events nearly simultaneously:

  • synchronize — from the force-push
  • edited — from the body update

Both resolve to the same concurrency group (PR Title Check-<pr number>), so cancel-in-progress: true cancels the first run when the second starts. Both runs had already created check-runs with identical names, leaving a dangling CANCELLED check-run alongside the successful one on the head commit.

The concurrency config works as intended for superseding older commits; it only backfires when two events land on the same commit.

Fix

The check only validates the PR title and description, and neither changes on a push. So synchronize is both semantically unnecessary and the specific trigger racing against edited. Trigger only on opened, edited, and reopened — the events that actually change what's being validated.

🤖 Generated with AI

The PR title check only validates the PR title and description, neither
of which changes on a push (synchronize). Including synchronize caused a
race with Dependabot rebases, which force-push and edit the PR body at
the same time, firing synchronize and edited together. Both events land
in the same concurrency group, so one run is cancelled, leaving a
cancelled check-run on the head commit that fails the status rollup even
though a successful run exists.

Trigger only on opened, edited, and reopened -- the events that actually
change the title or description.
@tokoko
tokoko merged commit 7460722 into substrait-io:main Jul 14, 2026
21 checks passed
tokoko pushed a commit that referenced this pull request Jul 20, 2026
## Problem

The **PR title / description conforms to semantic-release** check — a
required status check on `main` — stops being reported once a PR
receives a new commit, leaving the PR permanently `BLOCKED` (e.g. #224).
The check runs and passes when the PR is opened, but pushing a follow-up
commit never re-reports it.

## Root cause

GitHub keys required status checks to the **head commit** of the PR.
#203 dropped `synchronize` from the workflow triggers (to fix the double
check-run race in #198), so a push no longer runs the check. The passing
check-run stays attached to the *previous* head commit, and the new head
commit has no `PR title / description conforms to semantic-release` run
at all — so the status rollup can never be satisfied.

Dropping `synchronize` is fundamentally incompatible with this being a
required check: any contributor who pushes after opening a PR strands
the required check.

## Fix

- Re-add `synchronize` so the check reports on every head commit.
- Set `cancel-in-progress: false` so the #198 race doesn't return. That
race left a *cancelled* check-run because `synchronize` and `edited`
(fired together by a Dependabot rebase on the same commit) landed in the
same concurrency group and one cancelled the other. With cancellation
disabled the second event queues behind the first and both finish as
successes — two harmless `SUCCESS` check-runs instead of one `SUCCESS` +
one `CANCELLED`. The concurrency group is kept so runs for a PR still
serialize and the comment job can't race with itself.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants