Make any release failure turn the run red - #236
Closed
MelbourneDeveloper wants to merge 1 commit into
Closed
MelbourneDeveloper wants to merge 1 commit into
MelbourneDeveloper wants to merge 1 commit into
Conversation
The release pipeline had five ways to publish nothing and still report success, and v0.17.0 went out through all of them intact. - The win32 build and VSIX legs were `continue-on-error`, so a Windows binary that never built showed as a warning inside a green run. - The runtime archives were staged with `cp ... || true`, so a build producing none would package, publish and reach Homebrew as a compiler that can link nothing. - The publish tokens were probed, and a missing one skipped its channel with a warning: a release that reached neither Homebrew, Scoop nor Open VSX passed while `brew install` kept serving the previous version. - `git commit ... || exit 0` in the tap jobs swallowed every commit failure along with the nothing-to-commit case, and skipped the push. - The web compiler deploy was non-fatal by design, which is how the live playground served a stale build for weeks behind green releases. All five are now failures. `fail-fast: false` stays on both matrices, so a broken leg still lets its siblings finish and one run shows every platform's result — it just ends red rather than cancelling anything. That leaves the failure mode none of it addresses: GitHub scores a *skipped* job as green, so a wrong `if:` or an unset scope output publishes nothing and still succeeds. The new `release-complete` job runs `if: always()` after every other job and fails unless each channel this tag requires actually succeeded, driven by the same four `scope` outputs the jobs' own conditions use so the two cannot drift apart. Also removes the `|| true` hiding a failed `cargo install cargo-llvm-cov` behind a confusing error several steps later, and the one on the Android `sdkmanager` lookup, which is now a directory test with a real diagnostic. How the tests prove it works: `scripts/test-release-gate.py` extracts the backstop's shell out of release.yml — the real one, not a copy — and runs it against 13 fabricated outcomes: Homebrew silently skipped, a build leg failed, a job cancelled, a website-only tag whose site never deployed, a vsix-only tag whose Marketplace publish skipped, and a prerelease correctly leaving the live site alone. Six mutations of the gate were each confirmed to turn it red, including one that first slipped through and exposed a missing case: a cancelled job the tag does not require is caught only by the failure sweep, so that case now pins the sweep. `scripts/verify-release-gates.mjs` fails the PR that reintroduces any of this: `continue-on-error`, `|| true` or `|| exit 0` in a run step, a release job absent from `release-complete`'s needs, a renamed or deleted backstop, or a reviewed tolerance gone stale. Eight regressions were each confirmed caught, with the tree checksummed back to clean after every probe. The one surviving `|| true` — the first-release tag lookup — is in that script's reviewed list with its reason. Both run in `make lint` and in the already-required "Build, Format & Analyse" job, so no required-check list or ruleset changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
MelbourneDeveloper
enabled auto-merge (squash)
September 9, 2026 10:19
MelbourneDeveloper
disabled auto-merge
September 9, 2026 10:20
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.
TL;DR
The release pipeline had five ways to publish nothing and still report success. v0.17.0 went out through all of them intact. This makes every one of them a red run, and adds a backstop for the failure mode none of them covered: a job that never runs at all.
Details
A release is either published or it is not. These each downgraded "did not publish" to something quieter:
build/vsixmatricescontinue-on-erroron win32 — a Windows binary that never built was a warning inside a green runPackagestepcp compiler/lib/lib*.a ... || true— a build producing no runtime archives would package, publish and reach Homebrew as a compiler that can link nothingpreflightbrew installserved the previous versionbrew/scoopgit commit ... || exit 0swallowed every commit failure along with the nothing-to-commit case, and skipped the push with itdeploy-webcompilerAll five now fail the run. Nothing cancels:
fail-fast: falsestays on both matrices, so a broken leg still lets its siblings finish and one run shows every platform's result. It just ends red.The backstop. Removing the swallows does not address the failure mode that survives: GitHub scores a skipped job as green, so a wrong
if:, an unset scope output or a cancelled dependency publishes nothing and still succeeds. The newrelease-completejob runsif: always()after every other job and fails unless each channel this tag requires actually succeeded. It is driven by the same fourscopeoutputs the jobs' ownif:conditions use, so "ran" and "should have run" cannot drift apart.Two same-class defects outside the release path are fixed too: the
|| truehiding a failedcargo install cargo-llvm-covbehind a confusing error several steps later, and the one on the Androidsdkmanagerlookup, now a directory test with a real diagnostic.docs/RELEASING.mdgains a section stating the policy, and its secrets table is corrected — it namedVSCE_PAT,TAP_TOKENandSCOOP_BUCKET_TOKEN, none of which exist. The real credentials areBREW_SCOOP_PAT,OPEN_VSX_PAT,AZURE_CLIENT_ID/AZURE_TENANT_IDandFLY_API_TOKEN. It also still advertised adarwin-x64build leg that was dropped, and aSKIP_VSCE_PUBLISHvariable nothing reads.How Do The Automated Tests Prove It Works?
The backstop's logic is executed, not eyeballed.
scripts/test-release-gate.pyextracts therelease-completeshell body out of release.yml — the real one, so there is no second copy to drift — and runs it under bash against 13 fabricated sets of job outcomes:if:at all that did not run → failsSix mutations of the gate were each confirmed to turn the suite red: dropping the unconditional jobs from
required, no longer distinguishingskippedfromsuccess, no longer countingcancelledas broken, ignoring thewebsiteoutput, always exiting 0, and renaming the step out from under the test. Thecancelledmutation initially slipped through, which exposed a real gap — a cancelled job the tag does not require is caught only by the failure sweep, and no case covered that. Case 7 now pins it, and the mutation is caught.The swallows cannot come back.
scripts/verify-release-gates.mjsscans every workflow and fails oncontinue-on-error,|| trueor|| exit 0in a run step, on any release job missing fromrelease-complete'sneeds:, on a renamed or deleted backstop, on a backstop withoutif: always(), and on a reviewed tolerance that no longer matches anything. Eight regressions were each confirmed caught — including adding a plausible newpublish-snapjob and forgetting to wire it in — with the workflow tree checksummed back to identical after every probe. The single surviving|| true, the first-release tag lookup inrelease-change-detection.yml, is in the script's reviewed list with the reason it is safe; deleting the tolerance without deleting the entry fails the build.Both run in
make lintand as a step in the already-required Build, Format & Analyse job, so the pinned context list inverify-branch-protection.mjsand the ruleset stay in agreement — no new required check, no ruleset edit.actionlintis clean across all six workflow files.🤖 Generated with Claude Code