Repository navigation
CI/CD: cut the fixed per-PR toll — 63 checks, 117 runner-min, 20 min wall clock #15357
Description
Activity
Amplifier found while starting Task 1 — automated pushes also destroy the approval
Ruleset "Require review for external contributors" applies to
refs/heads/Dev_new_gui:required_approving_review_count: 1 dismiss_stale_reviews_on_push: trueThis is not in classic branch protection (
required_pull_request_reviews: null), which is why it
did not show up in the original measurement.Consequence: every automated push to a PR branch — the base-merge from
auto-update-pr-branches
(#15359) and the auto-fix bot commits (#15362) — does two destructive things, not one:- restarts all 63 checks (~20 min wall, ~117 runner-min), and
- dismisses the existing approval, sending the PR back to
REVIEW_REQUIRED.
Current state of the queue, all 5 open PRs:
PR mergeable mergeStateStatus reviewDecision #15356 MERGEABLE BLOCKED REVIEW_REQUIRED #15355 MERGEABLE BLOCKED REVIEW_REQUIRED #15354 MERGEABLE BLOCKED REVIEW_REQUIRED #15347 MERGEABLE BLOCKED REVIEW_REQUIRED #15343 MERGEABLE BLOCKED REVIEW_REQUIRED Nothing is blocked on a failing test. Every one is blocked on review, and the automation actively
re-blocks anything that gets reviewed. This raises the priority of #15359 and #15362 from
"wasted runner-minutes" to "the reason the merge queue does not drain".Correction + action taken
Correction to the comment above. Stale-review dismissal is not why the 5 open PRs read
BLOCKED. Checked/pulls/{n}/reviewson all five: none has ever had a review, so there was
no approval for a push to dismiss. Two further facts change the picture:- The ruleset carries
bypass_actors: [{actor_type: RepositoryRole, actor_id: 5, bypass_mode: always}]
— the repository-admin role bypasses the review requirement unconditionally. - The last three merges (fix(test): write the 11 vacuous tests for real and fix silent-failure helpers #15351, tech-debt(ansible): finish the role_*_active single-source collapse (#14678) #15349, test(perf): report a changed baseline as a state, not a regression #15342) all went in with zero approving reviews,
merged by the repo owner. The requirement is already bypassed in practice for every PR here.
So
mergeStateStatus: BLOCKED/reviewDecision: REVIEW_REQUIREDon this repo is cosmetic.
GitHub computes it without accounting for bypass actors, and it will keep reportingBLOCKEDno
matter what this ruleset says, short of removing thepull_requestrule entirely.The real cost is misdiagnosis, not blocked merges. Every tool, dashboard and agent that reads
mergeStateStatusto decide "can this land?" concludes the queue is jammed when nothing is
actually stuck. That is worth its own child issue under #15357.Change applied to ruleset
Require review for external contributors(id 17190144):dismiss_stale_reviews_on_push: true → falseUnchanged, deliberately:
required_approving_review_count: 1,
require_extra_approval_for_unattributed_changes: true, and the admin bypass — those still guard
the external-contributor case the rule is named for.This does not unblock anything today. What it does is stop #15359's base-merge commits and
#15362's auto-fix commits from discarding an approval once reviews do happen, so the fixes in
those issues no longer have to fight the ruleset as well.- The ruleset carries
#15358's fix as originally filed would have broken the required
verify-precommit-configcheck —check_gating_precommit_hooks.pytreats aSkippedgating hook as a failure, and its one allowlisted hook isfiles: \.sh$, notalways_run. Corrected design (split by hook class, not by file set) is on #15358. The measured saving stands; the mechanism changed.Two of my own children were wrong and are corrected:
- ci(perf): can the #9300 semantic-conflict net be had for less than a full 63-check re-run? #15359 restated a documented, already-decided design as a discovery.
auto-update-pr-branches.yml:33-49already recordsstrict: false(decision ci: strict up-to-date plus a single runner makes merging a race — 62-check runs outlast the ~31min gap between merges #13801) and states the refresh exists for the release: Dev_new_gui → main #9300 semantic-conflict net, with its own "WHY NOT REMOVE THE REFRESH ENTIRELY" section. Re-scoped to the narrower, still-open question: whether that net needs all 63 checks. The original "stop the base-merge" framing must not be implemented. - ci(tooling): mergeStateStatus reads BLOCKED on mergeable PRs — bypass actors are not accounted for #15367 closed as not-actionable here. Both repo matches for
mergeStateStatusare comments warning against using it (ci: auto-update-pr-branches passes but still updates nothing — mergeStateStatus is not computed yet at push time #12818, and the Self-hosted runners on Ubuntu 25.10 (non-LTS): actions/setup-python has no 3.14 build, blocking the self-hosted migration #15313-class incident note infrontend-required-context.yml:23) — no code to fix. The misreading lives in local tooling outside this repo.
Net effect on the umbrella: Wave 1 is #15358 (corrected mechanism), #15360 (de-risked), #15361 (coverage comparison posted). Task 8 is withdrawn.
The measured baseline is unchanged: 63 checks, 117 runner-minutes, 20 min wall clock on PR #15355.
- ci(perf): can the #9300 semantic-conflict net be had for less than a full 63-check re-run? #15359 restated a documented, already-decided design as a discovery.
Session close — state of this umbrella
No code was written. Nothing was committed, no branch or worktree was created, no PR opened. This umbrella is a backlog with a measured baseline, not work in progress.
Verified against the code
Child State #15358 Premise sound, mechanism corrected — original fix would have broken a required check #15360 De-risked against the #12986 canonical-filter guard — no conflict #15361 Coverage comparison posted; hardenedis a strict superset ofdocker-smoke-testNot verified — do not implement as written
#15362, #15363, #15364 each carry a caveat comment. Two of four Wave-1 siblings had wrong premises on inspection, so these are hypotheses with measurements attached, not agreed fixes.
Withdrawn
- ci(perf): can the #9300 semantic-conflict net be had for less than a full 63-check re-run? #15359 re-scoped — original framing restated a recorded decision (ci: strict up-to-date plus a single runner makes merging a race — 62-check runs outlast the ~31min gap between merges #13801, release: Dev_new_gui → main #9300)
- ci(tooling): mergeStateStatus reads BLOCKED on mergeable PRs — bypass actors are not accounted for #15367 closed — no repo-side code to fix
Applied outside the tracker
Ruleset
Require review for external contributors:dismiss_stale_reviews_on_push: true → false. Reversible with one API call.required_approving_review_count: 1,require_extra_approval_for_unattributed_changes: trueand the admin bypass are unchanged — whether to also disable the second one is an open decision.Blocked
All code work is blocked on the local worktree ceiling (12 live / ceiling 10), whose override needs an edit to a file gated by #15369. Neither is fixable from inside this repo by an agent.
Baseline to measure against
63 checks · 117 runner-minutes · 20 min wall clock · median queue 0s — PR #15355.
- added a commit that references this issue
on Aug 31, 2026 Measured from the consuming side: one session's worth of the toll
A day of continuous PR work against this repository, offered as evidence for the sub-issues rather than as a new finding.
The base-merge cost is not theoretical (#15359)
Every merge puts every other open PR behind, and the standing practice here is to re-level them all. Today that meant roughly twenty update cycles across 4-8 PRs, each buying a full re-run. At #15359's own measurement of 117 runner-minutes per re-run, that is on the order of 39 runner-hours spent re-proving unchanged code.
The decisive fact is that branch protection does not ask for it:
gh api repos/:owner/:repo/branches/Dev_new_gui/protection --jq '.required_status_checks.strict' falseVerified directly today. So the re-run cost buys nothing the repository requires. A narrower rule preserves the actual safety — update only when the base commit touches a file the PR also touches — which catches genuine semantic conflicts and skips the large majority that cannot interact.
The two-machine dependency is the other half (#15395)
Independently confirmed while triaging a bump:
code-qualityrunsruns-on: [self-hosted, Linux, X64]and consumes./.github/actions/setup-python-ci. Across a full session both runners reportedbusy=truecontinuously, andcode-qualitywas the single most common reason a PR sat at 10/10 required contexts without becoming mergeable.Two observations that may help #15395's case:
actions/cache@v6is already proven on that hardware. The bump is gated onrunner.environment == 'self-hosted', andcode-qualitycompleted success on the bumped head (9e28baa631). So a move off self-hosted is not blocked by an unverified action version.busy=truewith zero in-progress runs is not evidence of a wedge. I asserted that earlier in the session and was wrong — 44 runs completed within a minute of the claim. Recording it here because a false wedge diagnosis would justify restarting healthy runners, and it makes the next real detection easier to dismiss.
What compounds them
These two multiply rather than add: a base-merge triggers a full re-run, and the re-run's slowest required context is pinned to a pool of two. Fixing either alone leaves most of the wall-clock in place.
Not offered as a decision
Both #15359 and #15395 change what the repository trusts, and #15395 explicitly supersedes a premise (#15310/#15311) that was acted on earlier today. That is an owner call. This comment is only the consumption-side measurement, since the sub-issues were filed from the CI side and this is what the toll looks like from a queue trying to drain.
- added a commit that references this issue
on Sep 1, 2026 Task 1 delivered, Task 2 should not be built as written
Task 1 —
enforce-precommitscoped to the diff (#15358). Merged as76980d2e66(PR #15392).
Measured on its own run: the job went 1142s → 150s, the sweep step 1008s → 105s (cold
cache). The umbrella predicted −21 min; the realised saving is ~16.5 min off the critical path,
and the 105s figure should fall further once the new~/.cache/pre-commitcache is warm — nothing
cached hook environments before, so every run rebuilt all of them before checking a single file.#15358 stays open on AC3 alone (a seeded violation proving the #14181/#14878 guards still fail).
Task 2 — stop the unconditional base-merge (#15359) — recommend NOT building it. Its preferred
fix readsmergeStateStatus, whichauto-update-pr-branches.yml:166explicitly forbids
("#12818: do NOT filter on mergeStateStatus. It is computed LAZILY"), and gating the refresh on
conflicting would delete the semantic-conflict net rather than cheapen it — semantic conflicts
are by definition the ones that do not conflict textually.The cost that task targets is not a defect in that workflow. It is the price of eager,
speculative per-PR CI: branch protection hasstrict: false, so a PR's checks can be stale at
merge and are simply accepted, and the base-merge exists to cover that gap. A merge queue removes
the reason it exists. #14054 is the real fix for Task 2, and its triggers landed today
(b9effcf723, PR #15394) — all ten required contexts now fire onmerge_group. Retiring
auto-update-pr-branchesis #14054's AC4.Detail recorded on #15359 itself.
Two findings from doing Task 1
- ci: the SPDX gate could not fail — insert-license repairs the tree in the step before it, so it validated a tree already fixed #15398 — the SPDX gate could not fail.
insert-licenserepaired the tree in the step before
the check read it, so green meant "a hook fixed it in flight". 15 files had never carried a
header; SPDX/Apache header coverage drifts — add enforcement hook + backfill post-sweep files #9840 and chore(license): backfill SPDX headers on first-party infra tooling (#12175) #12203 both passed against a repaired tree. Diff-scoping stopped the repair
and the real state surfaced. The same shape may affect other post-sweep guards in that job,
and none has been audited. - ci: build a priority lane — a
priorityPR label routes jobs to a reserved runner instead of the shared queue #15397 — there is no way to prioritise a PR's checks. Relevant here because ci: make the required contexts reportable — merge_group triggers (#14054) + off self-hosted (#15395) #15394, the PR
that removes the self-hosted bottleneck, spent hours queued behind that bottleneck.
Measurement correction for this umbrella
This issue records "median queue delay 0s — capacity is not the bottleneck". That held when
measured and still holds as the general case, but it is not invariant: at 17:00 on 31 Aug the four
deepest queues were the four self-hosted-pinned workflows, 29 of 89 queued runs against two
machines, while hosted capacity ran 7 jobs against a ~20 ceiling. PR #15394 removed that
contributor entirely (#15395). Worth re-measuring the 0s figure now that it has.- ci: the SPDX gate could not fail — insert-license repairs the tree in the step before it, so it validated a tree already fixed #15398 — the SPDX gate could not fail.
- added sub-issues
on Sep 1, 2026
Problem
A one-line change pays a fixed 20-minute wall-clock / 63-check / ~117 runner-minute toll, and
that toll is re-paid several times per PR for reasons unrelated to the code under review.
Measured on PR #15355 (
gh pr checks) and the last 200 workflow runs:The bottleneck is fan-out and re-triggering, not runner capacity. Nothing in the pipeline scales
to diff size: 12 test shards,
code-quality(18m),security(14m) andpre-commit --all-files(21m) run identically for a one-line change and a 50-file refactor.
Tasks
Wave 1 — independent, highest leverage:
enforce-precommitto the PR diff instead of--all-files— ci(perf): scope enforce-precommit to the PR diff, not --all-files (21 min → <5) #15358 (−21 min)paths-filterjobs into one reusable workflow — ci(perf): collapse 15 duplicate paths-filter jobs into one reusable workflow #15360hardened-smoke-testagainstdocker-smoke-teston PRs — ci(perf): de-duplicate hardened-smoke-test against docker-smoke-test on PRs #15361 (−10 min runner)mergeStateStatusas landability — bypass actors ignored — ci(tooling): mergeStateStatus reads BLOCKED on mergeable PRs — bypass actors are not accounted for #15367Wave 2 — depends on Wave 1 landing:
*-required-contextshim pattern — ci(perf): restore deleted path filters via the required-context shim pattern #15363Acceptance Criteria
api-wiringjobdismiss_stale_reviews_on_pushdisabled (2026-08-31) — stops automated pushes discarding approvalsgh pr checkson a real PR and posted on its child issue