fix(ci): gate the release before the draft exists - #1711
Conversation
The v0.75.0 evidence gate rejected image digests that could never pass at that commit, and escaping it meant deleting the draft by hand and reverting the version commit under an administrator bypass. The gate now runs before anything is created: tag-images resolves digests, generates and verifies the evidence, and only then creates the draft it uploads the evidence to. A rejected release leaves no draft and no tag, so the version re-cuts from a commit that carries the fix with nothing to clean up first. The parent-version precondition stays exactly as it was — it is a good invariant, and it only bit us because failure left state behind. Release notes move to the job that creates the draft, assembled from the CHANGELOG.md it already checks out; only the schema-migration bit, which needs tags, still travels as an output. Both release asset uploads clobber, so re-running over a resumed draft replaces assets instead of failing on their names. The changeset freeze rules are lifted for a verified revert, which scripts/verify-revert.ts decides structurally rather than by title: every commit the pull request adds must record a revert trailer, name a commit already reachable from the base, and carry that commit's patch exactly reversed by patch id. Reverting a release restores MIGRATION.md and the pending changesets by construction, so the guard could never pass on that path — it has been bypassed three times for it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 50 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (6)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Approved automatically: @FelixTJDietrich is listed in the REVIEW_POLICY_MAINTAINERS repository variable, which the repository treats as satisfying the review requirement. See the review policy in docs/contributor/ci-cd.mdx.
📚 Documentation Preview
|
🧩 Storybook Preview
|
Description
The v0.75.0 evidence gate failed after the draft release already existed, and because release images
are promoted by digest and never rebuilt, that draft could never pass — escaping it took a manual
draft deletion plus an admin-bypassed revert (#1701). This takes the reorder, not the cleanup
job: the gate now runs before anything is created, so a rejected release leaves no state at all.
Fixes #1705
Which approach, and why
Reorder — the preferred option.
tag-imagesalready resolved digests, generated and verified theevidence, wrote the image lock, attested and signed it before touching the release; only
gh release uploadneeded a release to exist. Creating the draft immediately before that upload wasa local change, so the fallback cleanup job was not needed:
releaseno longer creates anything. It decides whether this commit cuts a version, enforces thepreconditions, and publishes the same outputs as before.
tag-imagescreates the draft as its second-to-last step, then uploads the evidence to it.CHANGELOG.mdthat job alreadychecks out. Only the schema-migration flag still travels as a job output, because deciding it needs
tags and history that
tag-images(depth 1) does not have. A job output, not an artifact: outputssurvive a partial re-run in which
releaseis not replayed, and the notes themselves would be a~120 KB output at current release sizes, close enough to the per-output limit to be worth avoiding.
gh release uploadcalls gained--clobber, so a re-run over a resumed draft replaces assetsinstead of failing on every name that already exists.
The
PARENT_VERSION must be publishedprecondition is untouched, including its "draft targets adifferent commit" sibling. Both are good invariants; they only bit us because failure left state
behind.
scripts/ci-contract.test.tsnow asserts both are still there, that the draft is createdafter the evidence verifier runs, that assets are uploaded after the draft exists, and that every
release upload clobbers.
Failure modes considered
release.ymlcannot be rehearsed, so these are argued rather than tested:releasejob still resumes a draft that targets this commit,tag-imagesreuses it instead of recreating, and the uploads clobber. This is the window the resume path was designed for — a flake at the same digests. A draft that genuinely can never pass still needs deleting by hand, but that window is now three jobs wide instead of the whole pipeline, and I deliberately did not add anif: failure()cleanup, because deleting the draft is exactly what would break the resume path for the flake casereleasesees a published release, setsreleased=false, everything downstream skipsgh release uploadfails on existing asset names--clobberreplaces themgit diffagainst the previous tag errors while deciding the migration warningReordering does not weaken any gate: nothing between the digest resolution and the draft creation
reads the release, and the draft is still published only after the seeded-upgrade and supported-host
gates pass. Attestations and image promotion are unchanged.
Revert exemption, and its threat model
verify-changesetsfreeze-guardsMIGRATION.md,.migration/*.mdand pending changesets. A revertof a version commit restores all three by construction, so the guard cannot pass on that path — it
has been admin-bypassed three times (#1686, #1691, #1701), and a guard that must be bypassed on a
known-good path teaches people to bypass it.
scripts/verify-revert.tsdecides the exemption structurally. The title and branch name arenever read. Every non-merge commit the PR adds over the base must:
This reverts commit <sha>.line — the trailergit revertwrites;git patch-id --stableover a--binarydiff.
Threat model:
revert: …,revert/xbranch): impossible, neither is an input.commit denies the exemption for the whole PR.
--binaryputs the payload in the hashed diff, so adifferent blob is a different patch id. Covered by a test.
trailer does not match the pattern, and the parent count is checked.
landed. It is an ancestor of the base, so it passed this same guard or was bypassed by an admin.
git did not write, a shallow clone that cannot see the reverted commit — all leave the guard in
full force, and the admin bypass remains as the escape hatch.
The check runs with
contents: read, no secrets and no network; it only reads git objects from thealready-checked-out clone (
fetch-depth: 0).It is verified against the real case:
verifyRevert("8ff1a912a", "27a698954")— the base and head of#1701 — returns verified, and the squash-merged commit on
main(whose body lost the trailer) doesnot.
Changeset
None.
verify-changesetsscopesSHIPPED_PATHStoserver,webappanddocker, excluding**/*.mdand tests. This PR touches.github/,scripts/anddocs/only, so no changeset isrequired and none is added.
How to test
CI covers this:
pnpm run format && pnpm run checkpass locally,scripts/verify-revert.test.tsadds 12
node:testcases over throwaway git fixtures (release-shaped reverts, reverts taken afterlater work landed, riders, unrelated commits, fabricated trailers, binary swaps, merge reverts, empty
ranges, and the
GITHUB_OUTPUTcontract), andscripts/ci-contract.test.tspins the new release jobwiring.
release.ymlitself cannot be exercised without cutting a release — the reasoning above isthe substitute.
Checklist