fix: the merge gate could not block a merge - #76
Merged
Conversation
`merge-gate` had the right shape — `needs:` on the other jobs — and a condition that made it inert: `if: github.event_name == 'pull_request'` rather than `if: always()`. A job whose dependency fails is skipped, and GitHub counts a skipped check as passing for branch protection, so the gate reported success in exactly the case it exists to catch. It was harmless only because this repository has no branch protection at all and nothing requires the check. Requiring it, which is the point of adding it, would have installed a gate that cannot block. Two more problems in the same job: Its `needs:` list named nine of the eleven other jobs. `build` was covered only transitively, through the four jobs that depend on it. `docs` was not covered at all — it could fail with the gate still green. Its body was fifteen `echo "✅ ..."` lines. Six of them — approval-gate invariant, Slack adapter discipline, HTTP-client discipline, secret inventory agreement, chart secret paths, vendored byte-identity — correspond to nothing in the dependency list, so the job asserted properties it had no way to observe. It verified nothing directly and printed a checklist that read as though it had. Now it depends on all eleven jobs, runs with `if: always()`, and delegates to the shared gate in nanohype/.github, which treats anything other than `success` as a failure, refuses an empty `needs:`, and reads this workflow back to confirm no job in it is unwatched. That last check is what would have caught the missing `docs` dependency on its own.
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.
merge-gatehad the right shape and a condition that made it inert.Skipped reports Success
It ran
if: github.event_name == 'pull_request'rather thanif: always(). A job whose dependency fails is skipped, and GitHub counts a skipped check as passing for branch protection — from the docs, "A job that is skipped will report its status as 'Success'. It will not prevent a pull request from merging, even if it is a required check."So the gate reported success in exactly the case it exists to catch. It has been harmless only because this repo has no branch protection and nothing requires the check. Requiring it — which is the point of having it — would have installed a gate that cannot block.
Two more, found while fixing that
Its
needs:named nine of the eleven other jobs.buildwas covered only transitively, through the four jobs that depend on it.docswas not covered at all — it could fail with the gate still green.Its body was fifteen
echo "✅ …"lines. Six of them — approval-gate invariant, Slack adapter discipline, HTTP-client discipline, secret inventory agreement, chart secret paths, vendored byte-identity — correspond to nothing in the dependency list. The job asserted properties it had no way to observe, verified nothing directly, and printed a checklist that read as though it had.Now
Depends on all eleven jobs, runs
if: always(), and delegates to the shared gate innanohype/.github(nanohype/.github#26), which:successas failure —skippedandcancelledincludedneeds:That last check is what would have caught the missing
docsdependency without anyone noticing it by hand.Verified locally: the gate now names 11 of 11 jobs, nothing unwatched,
if: always().