Promote commits to viable/strict on a schedule, not by commit count - #22064
Merged
Conversation
viable/strict only advanced from commits whose position in main's history happened to be divisible by 4. Every other commit was permanently ineligible, and the manual escape hatch for promoting one did not work, so a commit could sit outside viable/strict indefinitely and miss the nightly build cut from it. Two independent reasons the escape hatch did not work: 1. viable-strict-gate ran on every push and failed on commits outside the sample. The viable/strict updater reads one row per workflow run rather than per workflow, so pushing a ciflow/trunk tag afterwards added a second, successful row beside the failed one instead of replacing it. Any single failure is fatal, so the commit stayed ineligible forever. 2. The promote workflow pushed its tag using the default GITHUB_TOKEN. A ref pushed with that token does not start any workflow run, so the tag triggered neither the full CI it was meant to force nor the gate. Counting commits is also the wrong unit. When the repository is busy, four commits can be twenty minutes apart and full CI runs far more often than it needs to. When it is quiet, four commits can span two days. Measured over two weeks of history on main, the longest a commit waited for the next eligible commit was about 46 hours, which is long enough to miss a whole nightly build. Changes: - viable-strict-gate runs only on ciflow/trunk tags now. A plain push produces no run of it, so the updater reports the required check as missing and declines to advance. Nothing writes a failure, so no commit can be permanently poisoned. - promote-to-viable-strict runs every 4 hours and promotes the tip of main. It exits without doing anything when viable/strict already contains the commit, or when the tag already exists, so repeated ticks over a quiet period do not re-run CI. One tick sits several hours before the nightly branch is cut, so the last commits of a quiet night still reach that day's nightly build. - promote-to-viable-strict checks out with the pytorchbot token so its tag push actually starts CI, and its sha input is now optional. - Dropped the step that deleted failed gate runs. It could not help: the updater's data comes from webhook events and deleting a run does not retract the row it already produced. With the gate no longer running on push there is also nothing left to delete. The depth based sample in _ci-run-decision.yml is unchanged. It still decides path filtering for the workflows that trigger on push, and several of those only listen for their own ciflow tags, so removing it would drop their coverage on main.
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/22064
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 6ade808 with merge base cff6f4d ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
meta-cla
Bot
added
the
CLA Signed
This PR needs a
|
The promote job did not declare an environment, so it could never read GH_PYTORCHBOT_TOKEN. That secret is an environment secret on update-commit-hash, not a repository secret and not an organization secret, so a job that does not name the environment sees an empty string. Declare the environment, matching weekly-pytorch-pin-bump.yml and nightly.yml. Without this the job fails on every run, and since the gate no longer runs on push, viable/strict would stop advancing entirely. Move permissions from workflow scope to job scope and reduce to contents: read. The tag is pushed with the bot token, so GITHUB_TOKEN only needs to read the repository for checkout. Every other workflow in the repository that grants contents: write scopes it to a job. Only apply the "already in viable/strict" check when the workflow chose the SHA itself. 13272 of 13274 first-parent commits on main are already ancestors of viable/strict, so applying it to a hand-dispatched SHA turned almost every deliberate request into a silent no-op that still reported success. Correct three comments that did not match the code: - _ci-run-decision.yml claimed a commit that misses the depth sample still gets full CI once promoted. It does not. Only 4 of the 11 callers of that workflow also listen for ciflow/trunk tags. The other seven, cuda, cuda-windows, cuda-perf, metal, mlx, rocm and vulkan, hold 35 of the 56 jobs gated on is-full-run, and for them the depth sample is the only thing that ever produces a full run on main. - The notice printed after the tag push named the wrong checks. lint has no tag trigger and Build documentation only accepts version tags, so neither starts from a promotion; their rows come from the push to main. - The header offered promoting a release branch tip as a use case. update-viablestrict is configured with main-branch: main, so that can never advance viable/strict. Also warn in viable-strict-gate.yml that adding workflow_dispatch would make the assertion pass and publish a gate success for a commit whose CI was path filtered.
shoumikhin
marked this pull request as draft
August 23, 2026 18:56
shoumikhin
marked this pull request as ready for review
August 23, 2026 19:08
A second review pass found that the comments the previous commit added were
themselves inaccurate, and that one number in the PR description pointed at the
wrong conclusion.
_ci-run-decision.yml:
- The seven callers that do not re-run from a promotion tag do have ciflow
tags of their own, just not ciflow/trunk. Say that.
- The list of workflows that re-run from the tag omitted viable-strict-gate,
so the counts in the sentence did not add up.
- "35 of the 56 jobs" was a grep line count. Measured by parsing the job-level
if: expressions, it is 32 of 48.
- "would drop them from main entirely" overstated it. 20 of those 32 also have
a changed-files branch and still run when their own paths are touched. Only
the 12 in mlx.yml depend on the depth sample alone.
promote-to-viable-strict.yml:
- The notice implied all seven tag-started workflows gate advancement. Only
four of them are in the required set. Say which.
- Build documentation does trigger on tags, just on version tags rather than
ciflow/trunk. The old wording was simply false.
- Use the names the Actions UI shows for the three MSVC/RISC-V workflows.
- The permissions comment said GITHUB_TOKEN was needed for checkout. Checkout
uses the bot token, so no step uses GITHUB_TOKEN at all.
- nightly.yml sets environment conditionally, so it is not the same pattern.
weekly-pytorch-pin-bump.yml is.
- Record that the update-commit-hash environment allows main only, so this job
cannot be exercised from a pull request branch.
viable-strict-gate.yml:
- schedule would spring the same trap as workflow_dispatch. Name it.
PR description: the 4 hour wall clock alternative was credited with a 4 hour
worst case. Measured, it is 20.2 hours over 14 days and 28.3 hours over 30,
because a wall clock window only opens when a commit arrives and so promotes
nothing during a quiet night. That is the case this change exists to fix, so the
alternative does not replace it. Also noted that the 1.98x is an event count
rather than a cost: a promotion tag unlocks 16 of the 48 gated jobs, a
depth-sampled push unlocks all 48.
A third review pass measured the thing the previous description declined to measure, and the conclusion it had reasoned to was wrong in direction. The claim was that a promotion tag is the cheap kind of CI event, because it unlocks 16 of the 48 jobs gated on is-full-run while a depth-sampled push unlocks all 48. That inference does not hold: a tag does not start 16 jobs, it starts seven whole workflows over again, and most of their jobs are not gated on is-full-run at all. Measured on cff6f4d, which carries both a push run and a real ciflow/trunk tag run, so the two are directly comparable on one commit: the tag run cost 6046 runner-minutes and 91 percent of that repeated work the push had already done. A promotion is more expensive per event than a depth sample, not less. The description now says so, and gives the one defensible aggregate, about 1.19x on the total main CI bill, with the reason it is below 1.98x (full runs are a minority of the bill) rather than the reason previously given. Also corrected: - The gate comment said adding schedule was the same trap as adding workflow_dispatch. It is the opposite trap. _ci-run-decision.yml returns false for a schedule event, so the assertion would fail rather than pass and every tick would write a permanent failure row on the tip of main. - "Build documentation only runs on version tags" was false. It also runs on push to main, on pull requests, and nightly. Dropped the parenthetical; the surrounding sentence was already right. - update-viablestrict.yml passes main-branch: main, but the action does not declare that input and passes a hardcoded value, so the line has no effect. The header comment cited it as the mechanism. The conclusion it drew was right for a different reason, which the comment now gives. - vulkan's only ciflow tag is the shared ciflow/nightly, so "their own ciflow tags" was not accurate for all seven. - The depth sample is not the only thing that can run the mlx jobs on main; workflow_dispatch and ciflow/mlx also force them. Narrowed to "automatically on a push to main". - 30-day event counts were computed by truncating history and then grouping into 4 hour windows, which turns the first in-range commit of a partly elapsed window into a false window-first. Grouping first and filtering after gives 7.30 and 4.00 events per day, 2.21x and 1.21x. - "at most 4 h" was the cron spacing presented as a bound. It excludes CI wall clock and the updater tick, and GitHub can delay or drop scheduled runs. - "no commit can be permanently poisoned" overstated it. The gate no longer writes a failure row, but the other five required checks still run on push and can still leave one.
JacobSzwejbka
approved these changes
Aug 24, 2026
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
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.
PR #22064: Promote commits to viable/strict on a schedule, not by commit count
Problem
viable/strict only advanced from commits whose position in main's history happened to be
divisible by 4. Every other commit was permanently ineligible, and the manual escape
hatch for promoting one did not work, so a commit could sit outside viable/strict
indefinitely and miss the nightly build that is cut from it.
Two independent reasons the escape hatch did not work:
viable-strict-gateran on every push and failed on commits outside the sample. Theviable/strict updater reads one row per workflow run rather than per workflow, so
pushing a
ciflow/trunktag afterwards added a second, successful row beside thefailed one instead of replacing it. Any single failure is fatal, so the commit stayed
ineligible forever. A comment in
promote-to-viable-strict.ymlclaimed the updater"resolves multiple runs by taking the latest conclusion", which is not what it does.
promote-to-viable-strictpushed its tag using the defaultGITHUB_TOKEN. A refpushed with that token does not start any workflow run, so the tag triggered neither
the full CI it was meant to force nor the gate.
Counting commits is also the wrong unit. When the repository is busy, four commits can be
twenty minutes apart and full CI runs more often than it needs to. When it is quiet, four
commits can span two days. Measured over the last two weeks of main, the longest a commit
waited to become eligible was 56.7 hours; over 30 days it was 58.0 hours. Either
is long enough to miss a whole nightly build.
Changes
viable-strict-gateruns only onciflow/trunktags. A plain push produces no run ofit, so the updater reports the required check as missing and declines to advance.
The gate never writes a failure row, so it can no longer be the thing that poisons a
commit. To be clear about the scope of that: the other five required checks still run
on push and can still leave a permanent failure row, so this removes one source of
poisoning rather than all of them.
promote-to-viable-strictruns every 4 hours and promotes the tip of main. On ascheduled tick it exits without doing anything when viable/strict already contains the
commit, and on any run it exits when the tag already exists, so repeated ticks over a
quiet period do not re-run CI. One tick sits ahead of the nightly branch cut, so the
last commits of a quiet night still reach that day's nightly build.
promote-to-viable-strictchecks out with a bot token so its tag push actually startsCI, and its
shainput is now optional and defaults to the tip of main.comes from webhook events, and deleting a run does not retract the row it already
produced. With the gate no longer running on push there is nothing left to delete
either.
The depth based sample in
_ci-run-decision.ymlis unchanged, and must stay. Sevenworkflows re-run from a
ciflow/trunktag, but only four of them call_ci-run-decision.yml. The other seven callers (cuda, cuda-windows, cuda-perf, metal,mlx, rocm, vulkan) have their own ciflow tags but not
ciflow/trunk, so a promotion tagnever starts them, and they hold 32 of the 48 jobs whose
if:gates onis-full-run.Most of those 32 also have a changed-files branch and still run on main when their own
paths are touched. The 12 in
mlx.ymldo not, so for those the depth sample is the onlything that ever runs them on main.
Cost and the alternative that was considered
pull,trunk,Apple,Windows MSVC Build,Test RISC-V BackendandTest QNN Windows MSVC buildtrigger on both a push to main and aciflow/trunktag,and their concurrency keys differ, so neither run cancels the other. A promoted commit
therefore runs those workflows twice.
The obvious cheaper alternative is to make
is-full-runtrue for the first commit ineach 4 hour wall clock window, computed from committer timestamps, instead of every
fourth commit by depth. No tag, no new workflow, no token. Measured over main's history:
Figures are over 14 days. Over 30 days: 3.30 / 7.30 / 4.00 events per day, 1.00x / 2.21x
/ 1.21x, and longest observed waits of 58.0 h / 4 h plus CI time / 28.3 h.
Three things about that table need saying, because a quick reading of it gets all three
wrong.
The alternative does not actually fix the problem. Its worst observed wait is 20 to
28 hours, not 4, and its true worst case is unbounded. A wall clock window only opens
when a commit arrives, so during a quiet night it promotes nothing, which is exactly the
case the nightly build cares about. This change promotes the tip of main on a timer
whether or not anything new landed, so a commit is eligible one tick after it lands.
That is the difference between missing a nightly and not missing one, and it is the
reason to prefer this approach despite the cost.
The 4 hours is the cron spacing, not a guarantee. Add CI wall clock and up to 30
minutes for the next updater tick, and GitHub can delay or drop scheduled runs under
load. On the one real promotion tag I could measure, all four required tag-started
workflows were green 2.2 hours after the tag, and that figure includes a hand re-run of
a cancelled
trunkattempt, so it is not a clean unattended measurement. The 08:00 UTCtick is about 3.5 hours ahead of the nightly cut, so the margin is real but it is not
large.
The cost is worse than the event count suggests, not better. A promotion tag does not
just re-run the jobs gated on
is-full-run; it starts seven whole workflows over again.Measured on commit
cff6f4dd, which received a realciflow/trunktag: the tag run cost6,046 runner-minutes (latest attempt of each job, skipped jobs excluded), and about 85
percent of that by runner-minutes, 82 percent by job count, repeated work the push to
main had already finished. Per event, a promotion is more expensive than a depth sample,
not less.
Where that lands in absolute terms: roughly 4.07 promotions a day at about 6,000
runner-minutes each is about 24,600 runner-minutes a day added, against roughly 128,000
a day for main CI today. So about 1.19x on the total main CI bill. That is the honest
number, and it is smaller than 1.98x only because full runs are a minority of the bill,
not because the added events are cheap.
One further cost not in the table: a promoted commit produces roughly twice the job rows
the updater must find green, so it roughly doubles the exposure to a flake blocking
advancement.
If the cost is judged too high, the direction I would look first is not the wall clock
alternative but adding
ciflow/trunk/*to the seven callers that do not listen for it,so that a tag unlocks all 48 gated jobs instead of 16. A rough estimate puts that at
about 1.16x against this change's 1.19x, so the saving is real but small, and I have not
measured it properly. Note that it would not allow deleting the depth sample: the sample
guarantees at most 3 consecutive commits without a full run, whereas promoting only the
tip at each tick guarantees nothing within a window. The busiest 4 hour window in the
last two weeks held 16 commits.
Correction to an earlier version of this description
An earlier version said
GH_PYTORCHBOT_TOKENwas not set on this repository and askedfor it to be added as a repository secret. That was wrong on both counts.
The secret is set. It is an environment secret on the
update-commit-hashenvironment, alongside
UPDATEBOT_TOKEN. The real defect was in this change: the promotejob did not declare
environment: update-commit-hash, so it could not read the secretand would have failed on every run. That is now fixed, following the same pattern as
weekly-pytorch-pin-bump.yml.Adding it as a repository secret would also have been the wrong fix, because a repository
secret is readable by any workflow on any ref.
The earlier version also said
nightly.yml,cherry-pick.yml,ghstack_land.yml,apple.ymlandweekly-pytorch-pin-bump.ymlreference unset bot tokens. That was aconsequence of the same mistake and is not true. All five are correctly wired:
cherry-pick and ghstack_land use
GH_PYTORCHBOT_CHERRY_PICK_TOKENon thecherry-pick-botenvironment, weekly-pytorch-pin-bump usesUPDATEBOT_TOKEN, apple.ymlfalls back to
GITHUB_TOKEN, and nightly.yml already declares the environment. No actionis needed on any of them.
Test plan
Ran the promote script against a real clone with the push and the remote tag lookup
stubbed out, covering every branch:
shainput, which is the scheduled path: resolves the tip of main, passesvalidation and reachability, and stops cleanly on the already existing tag. Exit 0.
tag push instead of exiting early. This was a real defect. 13272 of the 13274
first-parent commits on main are already ancestors of viable/strict, so the old
behaviour turned nearly every deliberate dispatch into a silent no-op that still
reported success.
explicit error before anything is pushed.
bash -n.Known limitation
The
update-commit-hashenvironment's deployment branch policy allowsmainonly, sothe promote job cannot be dispatched from this branch and is first exercised after merge.
Everything except the tag push and the secret read is covered by the runs above.