Commit a08a759
authored
ci(binary-size): stop re-triggering on label changes (#15418)
## The bug
`.github/workflows/ci-binary-size-validation.yaml` subscribes to the
`labeled` and `unlabeled`
pull_request event types:
```yaml
on:
pull_request:
types: [opened, synchronize, reopened, labeled, unlabeled]
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true
```
So **any** label write starts a second binary-size run on the identical
commit, and
`cancel-in-progress` kills the one already in flight. A label change
cannot alter a bundle's
size, so there is nothing for the job to re-evaluate — the second run
just recomputes the same
answer and leaves a permanent `cancelled` check run behind on the
commit.
## Why it matters beyond wasted minutes
The cancelled check does not go away when the retry succeeds. Both check
runs stay attached to
the commit, so the commit's check rollup is `FAILURE` forever.
The PR risk grader reads that rollup. Its reversibility axis treats a
non-`SUCCESS` rollup as
`no_green_checks_tier`, which `.github/risk.json` sets to **R2** — so a
PR gets bumped to R2
because a label bot wrote a `size:*` label two seconds after CI started.
The chain, end to end:
1. `dosubot[bot]` writes a `size:*` label seconds after the PR opens or
is pushed to.
2. That `labeled` event re-triggers binary-size on the same commit;
`cancel-in-progress` cancels
the in-flight run.
3. The commit now permanently carries a `binary-size / CANCELLED` check
run.
4. The grader counts `CANCELLED` as a failing check, so the rollup is
not `SUCCESS`, so
reversibility is pinned at R2.
## Measured (re-verified for this PR, 2026-08-18)
| measure | value |
|---|---|
| binary-size runs, 30 days (created >= 2026-07-19) | 4,307 |
| ...concluded `cancelled` | **1,128 = 26.2%** |
| ...concluded `failure` | 39 = 0.9% |
| complete 2-day window (created >= 2026-08-16) | 819 runs |
| ...cancelled | 164 = 20.0% |
| cancels superseded by a later run on the **same head SHA** | **163 of
164 = 99.4%** |
| median gap to the superseding run | **2 s** (p25 0 s, p75 4 s) |
| distinct head SHAs in that window carrying **both** a `cancelled` and
a `success` run | 110 of 613 = **17.9%** |
Cancellations outrun real findings by roughly 29 to 1.
**Timed to the second on three live PRs** (label event → new run → prior
run cancelled):
- #15413 — `size:XS` at 21:31:18Z → new run 21:31:20Z, prior run
(started 21:31:17Z) cancelled
- #15414 — `size:L` at 21:35:47Z → new run 21:35:49Z, prior run
(21:35:45Z) cancelled
- #15412 — `size:L` at 21:02:03Z → new run 21:02:06Z, prior run
(21:02:01Z) cancelled
**A settled rollup showing the consequence.** Commit `ce9ec9b4d0`
(#15414, a `browser_tests/`-only
PR) — rollup state `FAILURE` over 85 contexts: 57 `SUCCESS`, 26
`SKIPPED`, and exactly one
failing-class context, `binary-size / CANCELLED`. That PR carries
`risk:R2`.
**Across the whole label population:** of the 112 PRs currently carrying
`risk:R2`, **17 (15.2%)
have a check rollup whose only failing-class context is `binary-size /
CANCELLED`** — #14546,
#15138, #15173, #15178, #15261, #15280, #15307, #15354, #15371, #15376,
#15380, #15388, #15402,
#15404, #15410, #15411, #15414.
## Why removing the trigger is the right fix, and not the concurrency
group
Keying the concurrency group on `github.event.pull_request.head.sha`
does **not** help: a label
write happens on the identical commit, so it lands in the same group and
still cancels. The
re-trigger itself is what has to go.
`binary-size` is **not** a required check for merge — the `ProtectMain`
ruleset requires `test`,
`lint-and-format`, `e2e-status` and `website-e2e`, and `CLA Check`
requires `cla-assistant`.
## Trade-off, stated plainly
The `allow-large-binaries` bypass is read from
`github.event.pull_request.labels.*.name` at the
job `if:`. With `labeled` removed, applying that label no longer
re-evaluates the check
instantly — it takes effect on the next `synchronize`/`reopened`. That
costs nothing today:
**`allow-large-binaries` has been applied to 0 PRs, ever**, and the
check does not gate merge, so
a stale red never blocks anyone.
`scripts/cicd/check-binary-size.sh:165` prints "add the
'allow-large-binaries' label to the PR"
on failure; that advice still works, it just lands on the next push
rather than instantly. I have
deliberately not touched that file — happy to reword it in a follow-up
if a reviewer wants it.
## This PR demonstrates the fix on itself
`pull_request` workflow definitions are read from the PR head, so the
change is already live
here. Timeline:
- 22:50:13Z — PR opened
- 22:50:17Z — binary-size run `32194526247` starts
- 22:50:19Z — `dosubot[bot]` applies `size:XS`, **two seconds into the
run** — the exact pattern
that cancels the run on every other PR
Result: **one** binary-size run on this commit, no second trigger, no
cancellation,
`binary-size pass 35s`. Compare #15413 / #15414 / #15412 above, where
the same two-second label
write killed the in-flight run and left a permanent `CANCELLED` on the
commit.
## Scope
This is half the loop. The other half is grader-side: `CANCELLED` is
counted as a *failing*
check in `Comfy-Org/github-workflows`
(`scripts/pr-risk/grade-pr-risk.sh`), which is what turns a
cancelled run into an R2. That fix has **not shipped** and is not in
this PR. So this PR removes
the largest source of the cancellations but does not fully close the
loop — a binary-size run
cancelled for any other reason would still red the rollup.
No other change to the file.1 parent 413d78b commit a08a759
1 file changed
Lines changed: 1 addition & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
7 | | - | |
| 7 | + | |
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
| |||
0 commit comments