CI: Harden the release pipeline after the v8.17.0 release - #1642
Conversation
Four changes, each from something that actually happened releasing v8.17.0. Pin the publish tree to the release commit. On the workflow_dispatch path actions/checkout takes main's tip, not the release commit, and everything downstream reads that tree: the build, lerna.json, the gitHead lerna stamps into the npm metadata, and the commit range auto release turns into notes. A re-drive after later merges would publish a tree the tag does not describe. Push release tag already resolved the Release v commit; the working tree now follows it. No-op on the push path, where HEAD is already that commit. Retry the npm publish. Provenance signing aborted partway through v8.17.0 with a Rekor 409, leaving @grafana/scenes published and @grafana/scenes-react not. The identical command succeeded minutes later on a manual re-run, so the retry is what a human was doing by hand. The registry, not lerna's exit code, decides completion: after a non-zero exit the step re-probes npm and only fails once packages are still missing after three attempts. Assert the GitHub release exists instead of trusting auto's exit code. auto release created the release, then failed commenting on a referenced issue (the app has pull_requests:write, not issues:write). The released plugin guards its PR-comment path with a try/catch but not the issue path, so every release closing an issue would fail the same way after doing everything right. A post-release failure is now a warning, while a missing release is still an error. Report the end state to the step summary. Both v8.17.0 attempts surfaced only a red X: one was half-published, the other fully released. The summary now lists each package, the tag and the release, and says plainly whether a re-run would change anything. Also fail Resolve when lerna lists no publishable packages, which would otherwise leave PUBLISH=false and skip the publish silently green.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1914893. Configure here.
ivanortegaalba
left a comment
ivanortegaalba
left a comment
There was a problem hiding this comment.
I tested with the agent, and we found some small things. Can you have a look?
| RELEASE_SHA="$(git rev-list -1 --grep "^Release v[0-9]" HEAD)" | ||
| if [ -z "$RELEASE_SHA" ]; then | ||
| echo "No 'Release v' commit reachable from HEAD" >&2 | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
--grep matches any line of a commit message, not just the subject, so a commit whose body contains a line starting with Release v matches too
Asserting on the subject after resolving should be enough
| RELEASE_SHA="$(git rev-list -1 --grep "^Release v[0-9]" HEAD)" | |
| if [ -z "$RELEASE_SHA" ]; then | |
| echo "No 'Release v' commit reachable from HEAD" >&2 | |
| exit 1 | |
| fi | |
| RELEASE_SHA="$(git rev-list -1 --grep "^Release v[0-9]" HEAD)" | |
| if [ -z "$RELEASE_SHA" ]; then | |
| echo "No 'Release v' commit reachable from HEAD" >&2 | |
| exit 1 | |
| fi | |
| case "$(git log -1 --format=%s "$RELEASE_SHA")" in | |
| "Release v"[0-9]*) ;; | |
| *) echo "Resolved ${RELEASE_SHA} is not a release commit" >&2; exit 1 ;; | |
| esac |
There was a problem hiding this comment.
Fixed, and taken a step further than the suggestion.
Reproduced it first in a scratch repo: a commit whose body contained Release v8.17.0 was wrong resolved AHEAD of the real release commit, so --grep returned entirely the wrong sha. Confirmed the subject assertion catches it.
Rather than assert-and-fail, candidates are now filtered on the subject:
git log --grep "^Release v[0-9]" --format='%H%x09%s' HEAD | awk -F'\t' '$2 ~ /^Release v[0-9]/ {print $1; exit}'
Same fail-closed guarantee, but it skips the decoy instead of blocking a legitimate release. Verified by extracting the rendered step out of the YAML and running it against that scratch repo — it picks the real commit.
Worth flagging: this bug is already live on main, since the merged Push release tag greps the same way. This PR fixes it there too.
| # Always runs, because the end state is least obvious exactly when a step | ||
| # above failed: v8.17.0 attempt 1 was half-published and attempt 2 was fully | ||
| # released but still red, and both looked like a bare red X. | ||
| if: always() && steps.version.outputs.version != '' |
There was a problem hiding this comment.
always() also runs on cancellation, so a cancelled run would write a summary and exit 1
| if: always() && steps.version.outputs.version != '' | |
| if: ${{ !cancelled() && steps.version.outputs.version != '' }} |
There was a problem hiding this comment.
Fixed, now !cancelled() && steps.version.outputs.version != ''. A cancelled run would have written a summary claiming the release was incomplete and then exited 1.
| if [ -z "$STATE" ]; then | ||
| if printf '%s' "$NPM_OUT" | grep -q 'E404'; then | ||
| STATE="**MISSING from npm**" | ||
| else | ||
| STATE="unknown (npm view failed)" | ||
| fi | ||
| INCOMPLETE=true | ||
| fi |
There was a problem hiding this comment.
npm view failure that is not an E404 sets INCOMPLETE, which reds the job and prints "re-run this job" for a release that may be entirely complete. That looks like the same false red this step was written to remove.
Would it make sense to keep the hard failure for a confirmed E404 and warn on the unknown case?
| if [ -z "$STATE" ]; then | |
| if printf '%s' "$NPM_OUT" | grep -q 'E404'; then | |
| STATE="**MISSING from npm**" | |
| else | |
| STATE="unknown (npm view failed)" | |
| fi | |
| INCOMPLETE=true | |
| fi | |
| if [ -z "$STATE" ]; then | |
| if printf '%s' "$NPM_OUT" | grep -q 'E404'; then | |
| STATE="**MISSING from npm**" | |
| INCOMPLETE=true | |
| else | |
| STATE="unknown (npm view failed)" | |
| echo "::warning::npm view failed for ${SPEC}, could not confirm it is published" | |
| fi | |
| fi |
There was a problem hiding this comment.
Fixed as suggested. Only a confirmed E404 sets INCOMPLETE; an inconclusive probe emits ::warning:: and leaves the job's own result alone.
You identified the actual flaw in it — reddening a complete release because npm was briefly unreachable is exactly the false red this step was written to eliminate.
| fi | ||
| echo "| npm \`${SPEC}\` | ${STATE} |" >> "$GITHUB_STEP_SUMMARY" | ||
| done | ||
| EXPECTED_SHA="$(git rev-list -1 --grep "^Release v${VERSION}" HEAD || true)" |
There was a problem hiding this comment.
The release commit is resolved three times, independently: here, in Check out the release commit, and in Push release tag. This passes even when that value is wrong
Would it make sense to resolve it once in Check out the release commit, write it to $GITHUB_OUTPUT, and have this step and Push release tag read that output? A wrong resolution would then stay a single point of failure instead of being confirmed twice
Also ${VERSION} goes into a regex unescaped, so 8.17.0 also matches 8x17y0
There was a problem hiding this comment.
Both fixed. The sha is now resolved once in Check out the release commit, written to $GITHUB_OUTPUT, and read by Push release tag, Create local tag and the report. rev-list --grep appears zero times now.
That also removes the unescaped ${VERSION} regex by deletion rather than by escaping — you were right that 8.17.0 matched 8x17y0, and now there is no second pattern to get wrong.
| INCOMPLETE=true | ||
| fi | ||
| if [ "$INCOMPLETE" = true ]; then | ||
| printf '\n**v%s is INCOMPLETE - re-run this job.**\n' "$VERSION" >> "$GITHUB_STEP_SUMMARY" |
There was a problem hiding this comment.
If only the tag is missing here, a re-run cannot help
Push release tag is gated on publish == 'true' || release == 'true', so on the re-run there is nothing left to publish or release and the tag step is skipped again.
Wondering if we have any way to solve it 🤔
There was a problem hiding this comment.
Good catch — that was genuinely unrecoverable. Resolve now computes a third gate:
TAG_SHA="$(gh api .../git/ref/tags/v${VERSION} --jq .object.sha)" || TAG_SHA=""
[ "$TAG_SHA" = "$RELEASE_SHA" ] && echo tag=false || echo tag=true
and both tag steps are gated on publish || release || tag. So a fully published release whose tag is missing or on the wrong commit now re-tags on a re-run instead of skipping forever.
- Resolve the release commit by SUBJECT, once. `git log --grep` matches any line of a commit message, so an unrelated commit whose body quoted a "Release v..." line resolved ahead of the real release commit. Candidates are now filtered on the subject, which skips the decoy rather than failing the release. This bug is already live on main, where `Push release tag` greps the same way. The sha is resolved once and exported, and the tag steps and the report read that output. A wrong value is now a single point of failure instead of being independently re-derived three times, and the version no longer goes into a regex unescaped (8.17.0 also matched 8x17y0). - Track the tag as its own gate. `Push release tag` was gated on publish or release, so a fully published release whose tag was missing or on the wrong commit would skip the tag step on every re-run, leaving it unfixable. - Report on `!cancelled()` rather than `always()`, so a cancelled run does not write a summary claiming the release is incomplete and then fail. - Only a confirmed E404 marks a package missing in the report. An unreachable registry now warns instead of reddening the job, which would have recreated the false red the report step exists to remove.
3 participants
Hardening for the release pipeline, driven by what actually happened releasing v8.17.0 — the first live run of the release-PR flow. Every change below traces to a real failure in that run, not a hypothetical.
Recap of the run: the release PR opened, was approved and auto-merged correctly, and
release-pr/canaryskipped exactly as designed. Thenpublishfailed twice — once half-published, once fully released but still red — and needed a manual re-run.1. Pin the publish tree to the release commit
On the
workflow_dispatchpathactions/checkouttakes main's tip, not the release commit, and everything downstream reads that tree: the build,lerna.json, thegitHeadlerna stamps into npm metadata, and the commit rangeauto releaseturns into notes.A re-drive after later merges would publish a tree the tag doesn't describe.
Push release tagalready resolved theRelease vcommit; the working tree now follows it. No-op on the push path, where HEAD is already that commit.This was a gap in the earlier
workflow_dispatchfix — that pinned the tag's SHA but left the checkout on the tip.2. Retry the npm publish
v8.17.0 aborted partway with a Rekor 409 (
TLOG_CREATE_ENTRY_ERROR), leaving@grafana/scenespublished and@grafana/scenes-reactnot. The identical command succeeded minutes later on a manual re-run — so the retry is precisely what a human was doing by hand.The registry, not lerna's exit code, decides completion: after a non-zero exit the step re-probes npm, treats "everything is on npm" as success, and only fails once packages are still missing after three attempts. Safe because
from-packagerecomputes the publish set from the registry each call.Deliberately not applied to
Create GitHub release—auto releasecallsrepos.createReleaseunconditionally, so an in-job retry would 422 once the release exists and turn a good release permanently red. Its re-entrancy correctly comes from thereleasegate across runs.3. Assert the GitHub release exists, don't trust
auto's exit codeauto releasecreated the release and commented on 10 PRs, then hit issue #1621 with403 Resource not accessible by integrationand failed the step — after the release was already correct.This is not a one-off. In
@auto-it/released, the PR-comment path is wrapped intry/catchbut the issue-comment path is not, so any release containing aCloses #NPR fails the same way. A post-release failure is now a::warning; a genuinely missing release is still an error.Granting the app
issues: writeis still worth doing, but it's now a quality-of-life fix rather than a blocker.4. Report the end state to the step summary
Both v8.17.0 attempts surfaced only a bare red X — one half-published, one fully released. Working out which took reading ~2,000 log lines. The summary now lists each package, the tag (and whether it points at the right commit) and the release, then says plainly whether a re-run would change anything.
Also:
Resolvenow fails iflerna listreturns no publishable packages, which would otherwise leavePUBLISH=falseand skip the publish silently green.Verification
The release path can't be exercised by CI, so this is verified by targeted local testing rather than a live run:
publishhas the expected 13 steps;canaryuntouchedf52184deon main — the correct v8.17.0 commitspecsoutput format verified as space-separatedHonest caveat: the real test is the next release. These paths only execute during a publish. The design intent is that every change either fails closed or is a no-op on the happy path.