ci: gate a release tag on a green dependency-floor run - #252
Merged
Merged
Conversation
_checks.yml runs the floors only for scheduled.yml, never for pull requests, so a floor break can merge with every check green and be tagged minutes later. That is how the crash in 1.8.0 reached PyPI. The gate requires a successful scheduled.yml run whose commit contains the tag, rather than merely the most recent successful run: when 1.8.0 was tagged the most recent run was green, having predated the commit that broke the floor. Closes #245
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.
Closes #245. Option 4 from the issue, without the option 2 narrowing.
Why the literal proposal would not have worked
The issue proposes "do not tag unless the most recent scheduled run is green". Checked against the
release it was written about, that gate waves 1.8.0 straight through:
It was green because it predated the commit that broke the floor. Recency of the run says nothing
about whether it covered the code being tagged.
So the gate asserts the stronger thing: a successful
scheduled.ymlrun whose commit contains thetagged commit, i.e. the tag is an ancestor of (or equal to) the commit the floors ran on. That
makes "green" mean "green on this code" rather than "green on something".
Verified against real history
Three cases, run locally against the actual run list:
HEAD= 1.9.2 commit, real run liste4c844e, dispatched before that tagHEAD= 1.8.0 commit, run list as it stood when 1.8.0 was tagged83730ca,dd03c41,1694963The second case is the one that matters: the gate retroactively blocks the release that motivated the
issue, against exactly the data that existed at the time.
Shape
floors-gateruns beforerelease, so a block costs nothing: PyPI has not been touched. Theexisting ordering comment in
release.yml(PyPI first, GitHub Release second) is unchanged, and itsnow-false claim that there is "no in-workflow CI gate" is corrected.
A run dispatched by hand counts the same as a scheduled one. That is deliberate: it is the documented
escape hatch, and it is what I did before tagging 1.9.2. The error message spells it out, including
the re-push:
There is no bypass flag. Dispatching the floors run is the bypass, and it takes about 13 minutes.
Permissions moved from one repo-wide block to per-job least privilege: the gate gets
contents: read+actions: read, the release job keepscontents: write+id-token: write.What this does not do
Option 2 (narrowing the floors job so it can run on pull requests) is not included, per the
instruction to do option 4 only. The consequence is unchanged: a floor break still merges green and
is caught at tag time rather than at review time.
The gate also cannot prove the resolution is still green today, only that it was green on this
code. The floors job resolves direct dependencies at their floor and transitives at their newest, so
a sufficiently old covering run could pass the gate while today's resolution would fail. With the
daily cadence from #233 the covering run will normally be under a day old, so no max-age check was
added; if that assumption stops holding, this is where it would go.
Testing note
mapfileis bash 4+, absent on macOS's bash 3.2, so the loop useswhile readinstead. That is notcosmetic: it is what made the three cases above runnable locally rather than only on a runner.