ci: add a single required CI gate job - #52
Merged
Merged
Conversation
master has no branch protection, so nothing mechanically stops a red PR being merged - it has rested on checking CI by hand, and a masked exit code nearly got a red run reported as green today. The gate aggregates every job so protection can require ONE stable check: requiring 29 matrix contexts directly means editing the rule whenever the matrix changes, and a renamed cell would silently stop being required. Two properties red-proofed before wiring it: - results logic (bash): all-success passes; failure, skipped, cancelled and EMPTY all fail - no results must not read as fine - completeness: dropping tmux-status-rows from `needs` makes the gate fail, so a job added later cannot sit outside it while CI stays green
GitHub expressions reject double-quoted strings, so `join(needs.*.result, " ")` made the whole workflow invalid: the run concluded in 0s with no jobs and no pull_request run was created. actionlint did not flag it.
Bisected the invalid-workflow failure: a minimal ci-gate job was accepted, and `join(needs.*.result, ' ')` was accepted too, so the fault lay in the parts stripped in between - the trailing two-line comment after `if: always()` being the remaining suspect. Comments now sit on their own lines above each key. The completeness assertion moves to tests/ci-gate-covers-all.py: tracked, runnable locally, and red-proofed (dropping install-nosudo from needs makes it exit 1 naming that job) rather than buried in a YAML heredoc.
GitHub scans the whole workflow for expressions, comments included, so the
literal `${{ }}` in a comment was an empty expression and made the file
invalid - the second of two faults, introduced while documenting the first.
An invalid workflow file runs nothing, so CI cannot report the fault - and once branch protection requires `CI gate`, an invalid workflow means that check never reports and the PR blocks with no visible cause. This class has to be caught locally, which is why it is a script rather than a job. Catches both faults that cost runs on PR #52: a double-quoted string inside an expression, and the expression delimiters written in a comment (GitHub scans comments, and an empty pair is a syntax error). Red-proofed by reintroducing each into a copy of the workflow - both exit 1; the clean tree passes 3/3. Reports a skip by name when actionlint is absent, so a pass never claims more coverage than it checked.
Owner
Author
|
Added after the initial description: Two genuine faults were found in this PR's own workflow, both invalid-workflow errors:
Diagnosis was slow because the signal is poor: 0s run, zero jobs, no |
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
2 participants
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.
What
A
ci-gatejob ("CI gate") thatneedsevery other job and fails unless all of them succeeded. Intended as the single required status check for branch protection onmaster. Folds inv1.11.7.Why
masterhas no branch protection at all (verified:GET /branches/master/protection→ 404, no rulesets). Nothing mechanically prevents merging a red PR — it has rested entirely on me checking CI by hand, and earlier today a trailingechomasked a watcher's exit code and nearly had me report a red run as green. That is a rule that has already been broken once, so it wants a control rather than more care.Requiring the 29 matrix contexts individually would mean editing the protection rule in lockstep with the matrix, and a renamed cell would quietly stop being required. One aggregate check is stable.
Verified
Both properties red-proofed before wiring, since a gate nobody has watched fail is decoration:
successfailureskippedcancelledThe empty case was a real bug found by the proof: the first draft passed vacuously when no dependency results arrived. "Cannot tell" must not read as "fine".
Completeness assertion, run against a mutated workflow: dropping
tmux-status-rowsfromneeds→FAIL - not required: tmux-status-rows; unmodified →PASS - gate requires all 6. So a job added later cannot sit outside the gate while CI stays green.if: always()is deliberate — without it the gate would be skipped when a dependency fails, and a required check that never reports leaves the PR hanging rather than failing.Follow-up (not in this PR)
Once merged, branch protection on
mastergets: required status checkCI gate, PRs required (0 approvals, single maintainer), no force pushes, no deletions, enforced for admins — so the gate binds the owner too, which is the point.