feat(release): one button that bumps, builds, verifies and publishes - #234
feat(release): one button that bumps, builds, verifies and publishes#234GuyMoses wants to merge 14 commits into
Conversation
Releasing was: run a macOS-only script, open a PR, merge, then remember to push a tag. Then it became: prepare, merge, and the merge triggers a build. Both left a window where main named a version whose binaries did not exist — unbounded in the first, 57 seconds measured in the second. The workflow now bumps the version itself, so the order can be inverted: everything expensive happens while main still points at the old version, and main only learns the new one once the binaries are already on GitHub. Check out, bump and tag locally, build into a draft, verify it, push main, push the tag, publish. Steps five to seven are two API calls. Pushing to main needs an identity the ruleset lets through. GitHub Actions' own token cannot be granted that — GitHub refuses it as a bypass actor with a 422, tested on a throwaway repo — so the Dash0 Release Bot App is named in the bypass list and used for that one step. Everything else keeps the built-in token. Verified end to end: the App's token pushed to protected main while a plain GITHUB_TOKEN push was rejected with GH013 seconds earlier. A concurrent merge is safe by construction. The push is a plain one, so it is rejected if main moved since the plan checked it out, and nothing is published before that point — a lost race costs one build and no exposure. The only way to reach the publish step is to have won it. A run that fails after pushing main leaves a version with no release. Dispatch again and the planner sees main already carrying an unreleased bump and finishes that one, rather than bumping past it and skipping a version permanently. This also answers the review objection to the previous shape: with no merge to detect, the branch trigger, the mode=none path and the pinned-vs-published comparison all disappear. What is left is what to tag and whether it claims Latest. scripts/release.sh is deleted — the workflow does this from a clean checkout of main, and a second path only drifts. release-prepare is not carried over. 19 contracts in test/contracts/release-plan.sh, since the workflow cannot be dispatched from a PR and so is never exercised before merge.
|
Darkplane auto-approval is enabled for this repository (mode: Dry run). An evaluation will run once this pull request's CI checks have completed — no action needed. |
|
Darkplane would not approve this pull request. It scored 24.93 of 100, and 75 was required.
An AI evaluator produced these scores from the diff, the repository history, and the pull request text. The pull request text is author-controlled, so treat the scores as signals rather than proof. Reviewed for commit 59847a0. |
CI's shellcheck flags SC2015 — C runs when A is true, which is not what the chain meant. The if says it plainly.
Six findings from review. The first is the one that matters. `channel=stable` accepted a prerelease in the `version` input and wrote it to main. Reproduced: IN_VERSION=0.2.0-dev.9 on stable yields bump_needed=true, so the run commits that to main — and since the Claude marketplace lists this repo with no ref, every subsequent install pins the prerelease. `latest=false` keeps GitHub's Latest pointer on the last stable, so nothing else would have signalled it. Prereleases now have exactly one route: channel=dev. A stable release cut while main already pins a prerelease is refused for the same reason. That also removes the only way `higher()` could be handed a prerelease, which it mis-ranks — `sort -k3,3n` reads `0-dev` as `0` and falls back to a lexical tiebreak, putting 0.2.0-dev.1 above 0.2.0. Both sides are plain X.Y.Z now: PUBLISHED excludes prereleases by construction and the two guards rule them out on the other side. Noted where it matters. The recovery branch silently discarded an explicit version: asking for 0.2.0 while main carried an unreleased 0.1.26 published 0.1.26 without a word. Refused now, naming both. The end-to-end job could go green having installed the PREVIOUS release. It only checked that some binary existed, so an empty DASH0_VERSION — a bare dispatch, a dropped plan output, a caller omitting `with:` — fell back to the version pinned in the checkout and reported the new release as verified. It now asserts the cache filename carries the version under test. Reusing an existing tag assumed it sat at HEAD, but when bump_needed=true HEAD has moved to the bump commit the planner never saw. Now checked, and refused when it points elsewhere. `make version-check` was referenced by nothing — CI calls the script directly. Wired into `make lint`, so a pin drift fails locally too.
| @@ -76,29 +76,27 @@ go test ./... | |||
|
|
|||
| ## Package | |||
There was a problem hiding this comment.
suggestion: remove this section in the cursor specific docs.
There was a problem hiding this comment.
Done, cut to three lines pointing at DEVELOPMENT.md. Kept the DASH0_VERSION paragraph — it is the only cursor-specific thing on the page. Want that moved too?
| - name: Bootstrap download contracts | ||
| run: ./test/contracts/bootstrap.sh | ||
|
|
||
| - name: Release planning contracts |
There was a problem hiding this comment.
issue: release-plan should not be executed on every ci run
There was a problem hiding this comment.
Fair, wrong job. Moved to consistency-checks, next to version.sh check
| # Runs after a release is published. Tests the full flow: claude-on-event.sh | ||
| # downloads the real binary from GitHub Releases, invokes it with events, | ||
| # and verifies the mock OTLP server receives the expected requests. | ||
| # The full published-release flow: claude-on-event.sh downloads the real binary |
There was a problem hiding this comment.
question: are these post release tests really necessary? They don't run an actual agent and release.yml already checks artifacts. IMHO I would remove e2e-release.yml altogether.
There was a problem hiding this comment.
Removed the workflow, kept the test as a job in release.yml.
| # plan checked it out, and nothing has been published yet — so a lost | ||
| # race costs one build and no exposure. This is also the only step that | ||
| # uses the App's identity. | ||
| if ! git push origin HEAD:main; then |
There was a problem hiding this comment.
though: This should kick off the ci pipeline. Await the build before publishing the release? It checks if all artifacts of the given version are published, which they are not here.
| echo "Published https://github.com/${GITHUB_REPOSITORY}/releases/tag/${TAG}" \ | ||
| >>"$GITHUB_STEP_SUMMARY" | ||
|
|
||
| - name: Every bootstrap resolves a binary at its public URL |
There was a problem hiding this comment.
question: is this necessary as the steps before verified the release artifacts already?
There was a problem hiding this comment.
Different question. The earlier step asks the GitHub API which assets are attached to the release id, while it is still a draft. This asks releases/download/v0.1.26/<name> over HTTP — anonymously, as a bootstrap does — for every name each of the seven bootstraps can construct, including the PowerShell ones.
| # GitHub for a release that was never tagged, and since the Claude marketplace | ||
| # lists this repo with no ref, that reaches users on their next `plugin install`. | ||
| # This is the only list of them, so the bump and the check cannot disagree about | ||
| # what needs bumping. Used by .github/workflows/release-prepare.yml, CI's |
There was a problem hiding this comment.
nit (automated review finding): stale references to .github/workflows/release-prepare.yml, which does not exist on main and is not added here. Line 88 in this file has the same problem ("release-prepare then dies at git commit -a" — that is release.yml now).
Two more of the same class, since the PR body says release.sh is deleted:
scripts/verify-release-assets.sh:18justifies non-strict mode with "CI on a bump PR, where the tag comes only after the merge". There are no bump PRs any more.test/consistency/copilot_test.go:131still creditsrelease.shwith keeping the versions in sync.
On the upside: I checked every version pin in the repo against the MANIFESTS + BOOTSTRAPS lists here, and the ten-pin list is complete — nothing escapes check.
There was a problem hiding this comment.
fixed — all four: version.sh:16 and :88, verify-release-assets.sh:18, copilot_test.go:131.
| uploaded asset list matches what was built. A failure here publishes nothing. | ||
| 5. Push the bump to `main`. | ||
| 6. Push the tag, then flip the draft to published. | ||
| 7. Check every public download URL, then install the real binary end to end. |
There was a problem hiding this comment.
nit (automated review finding): numbering this as step 7 of the ordered flow reads as though it is part of the gate. It is not — by this point the release is public and the tag is pushed. Steps 1 to 4 gate; 5 onward are the commit.
Same list: "Steps 5 to 6 are two API calls" — git push is not an API call, and the PR description says "five to seven" for the same thing.
There was a problem hiding this comment.
fixed — the list now says steps 1 to 5 publish nothing and a failure in any of them deletes the draft, step 6 is where the release becomes real, step 7 is the check that it did. Dropped the "two API calls" claim about the pushes.
| # binary catches an artifact that is complete and correctly checksummed | ||
| # but not actually executable on its platform. | ||
| ( cd dist && sha256sum -c checksums.txt ) | ||
| N=$(find dist -maxdepth 1 -name '*-on-event-*' | wc -l) |
There was a problem hiding this comment.
nit (automated review finding): no -type f, so this counts directories too. Correct today only because every build in .goreleaser.yaml sets no_unique_dist_dir: true and the binaries land in dist/ root. Drop that setting and GoReleaser's per-target directories inflate N past 16. Same line at 111 in the dry-run job.
There was a problem hiding this comment.
Gone — the count is a name-by-name diff now. scripts/expected-artifacts.sh derives every expected binary from .goreleaser.yaml, so a missing target names itself instead of printing 23 != 24. The count was already stale: Windows took it to 24.
| VERSION="${IN_VERSION:-$PINNED}-dev.${RUN_NUMBER}" | ||
| TAG="v$VERSION" | ||
|
|
||
| else |
There was a problem hiding this comment.
nit (automated review finding): any CHANNEL that is not dev falls through to the stable path. The type: choice input constrains the dispatch UI, not an API dispatch or a future caller, so a typo silently plans a stable release. The guards below limit the damage, but a case with an explicit *) die would be clearer about intent.
There was a problem hiding this comment.
Moot — the dev channel is removed, so there is no CHANNEL input left to typo.
| # tagged tree keeps pinning the base version — a dev build is consumed through | ||
| # DASH0_VERSION, not by installing that tag. | ||
| MODE=release; BUMP_NEEDED=false | ||
| VERSION="${IN_VERSION:-$PINNED}-dev.${RUN_NUMBER}" |
There was a problem hiding this comment.
nit (automated review finding): channel: dev with version: 0.2.0-dev.3 produces the tag v0.2.0-dev.3-dev.41. It passes the regex at line 51 and tags something nonsensical. Refusing a prerelease in IN_VERSION on the dev channel too would be consistent with the stable guard at line 76.
There was a problem hiding this comment.
Moot — same, the dev channel is gone.
# Conflicts: # test/contracts/README.md
From review. The override introduced by this PR was unvalidated in all four bootstraps, and VERSION feeds both BASE_URL and BINARY. curl squashes `..` in a path, so `v../../../owner/repo/releases/download/v9` retargets the download at another repository — and checksums.txt is fetched from the same BASE_URL, so verification passes against the attacker's own manifest before the binary is exec'd. $BINARY traverses the same way and writes outside BIN_DIR. The threat model fits this product: the hook runs inside an agent session, so an injected instruction that writes DASH0_VERSION into a project .envrc or a shell profile reaches it. Nothing on main is affected — the override arrives with this PR. Reuses the regex release-plan.sh and version.sh already carry. Verified: traversal, a leading `v`, and a shell-injection attempt are all refused with nothing written; 0.1.26 and 0.1.26-dev.7 still work. The contract fails without the guard and passes with it. It also unsets DASH0_VERSION for the whole suite — the contracts derive expected cache paths from each script's pinned VERSION, so a developer with it exported was getting a false failure.
…nnel Review found that dispatching this workflow was equivalent to write access on main. workflow_dispatch runs the workflow file *from the branch it is dispatched from*, so anyone with write access could push a branch carrying an edited release.yml, dispatch it, mint the App token and force-push anywhere. The REF_NAME check in release-plan.sh lives in the same tree they control, so it was never a barrier. That invalidated the argument I made when asking for the App — "workflows are code behind a reviewed PR" is false for workflow_dispatch. The bypass was a general branch-protection escape for everyone with write access, bought to save 57 seconds a month. The credentials now live in a GitHub Environment restricted to main, and the repo-level copies are deleted. A job declaring the environment on another ref is rejected before it starts; a job that drops the declaration finds nothing to read, because these are no longer repo secrets. That check is GitHub's, keyed to the ref, so it cannot be edited around. Also: the token is minted only when there is a bump to push, the checkout no longer uses it, the push sets the remote explicitly for that one command, and the job has a timeout now that it holds a credential. The dev channel is removed. Cutting from a feature branch and gating the credential by branch are mutually exclusive without splitting the push into its own job — the environment is restricted to main, so the job cannot run elsewhere. Stable and dry_run both run from main and are unaffected. Adding dev back means restructuring the job graph, which is worth doing on its own rather than inside a PR with 22 open threads. That also removes two outstanding review nits for free: `channel: dev` with a prerelease version produced the tag v0.2.0-dev.3-dev.41, and any CHANNEL that was not "dev" fell through to the stable path.
…every failure The e2e job took its version from a workflow_call input. An omitted or empty one skipped the check entirely, and the bootstrap then fell back to the version pinned in the checkout — the OLD one — while the job reported the new release verified. Folding the job into release.yml removes the input: the version comes from the plan and cannot arrive empty. The match is exact too; as a substring 0.1.2 was satisfied by on-event-0.1.25-linux-amd64. Also here: - The draft was deleted on the push-failure path only. A failure anywhere after the upload left a draft behind, and the asset diff takes the newest draft for the tag — so the next run would have compared against the stale one. Cleanup now runs on any failure, and only while the release is still a draft. - The public asset check is retried. The release stopped being a draft seconds earlier, and a transient 404 there fails a release that is already published. - The repo is no longer hardcoded in version.sh and verify-release-assets.sh; on a fork, --strict was validating upstream's assets. - A positive contract for `version.sh set`, the one command that rewrites the ten pins. Verified failing when a sed stops matching. - A failed push is no longer reported as "main moved" — that is the usual cause, not the only one. - `find -type f`, so a directory matching the glob cannot pass the count. - Norbert's three: the cursor README's duplicated artifact table is gone, the e2e workflow is folded in, and test/contracts/release-plan.sh is release.sh.
Windows support landed on main and the release path had to absorb it:
- `scripts/version.sh` now owns thirteen pins, not ten — the three PowerShell
bootstraps pin the version in their own syntax, and `scripts/release.sh`
(deleted here) was the only thing that knew about them.
- `scripts/verify-release-assets.sh` probes windows-amd64/arm64 and the `.ps1`
bootstraps, resolves `${AGENT}` and re-applies `${EXE}` per platform. It skips
Windows only while the release under test predates it, decided by asking that
release's own checksums.txt — so the skip retires itself at 0.1.26 rather than
on a hardcoded cutover.
- The build check no longer counts. "expected 16" was already wrong the moment
Windows landed, and a count cannot name the target that went missing.
`scripts/expected-artifacts.sh` derives every name from `.goreleaser.yaml` and
the workflow diffs `dist/` against it.
Conflicts were the DASH0_VERSION block sitting where main added its Windows
OS/EXE detection (both kept), and CI's inline asset probe, which this branch had
already moved into a script.
It ran 60 times between 14 July and 31 August and caught nothing. Its one red run (c35bccc, 18 Aug) was a GitHub API rate limit, not a defect. Every job in it had already run on the PR, and since the Windows matrix landed it spends the live Claude, Codex and Copilot canaries twice per merge. It mattered for the release workflow specifically: the bump push to main uses the App's installation token, which — unlike GITHUB_TOKEN — does trigger workflows. Proven by the probe push 6462ab2, where CI started four seconds later. That run can see nothing new; the tree is the one CI just validated plus 13 version pins that no Go code reads. What the trigger covered in principle is a PR merged on a stale base, which the ruleset permits (strict_required_status_checks_policy: false). The fix for that is to require the checks and turn strict on, not to watch trunk afterwards. The e2e gate loses its now-dead `push` clause. Inverting the pull_request test keeps fork PRs out for the same reason as before — they get no secrets — and lets a manual dispatch reach the canaries, which the old form did not.
… key The manifest rewrite is a blanket s/"version": "…"/, so a second key named "version" anywhere in a plugin.json would be moved with it. `check` compares only the keys pins() knows about, so it would still pass and the release would ship the change. Counting the keys first makes that a failure with a name. Not switched to jq: it reformats four of the five manifests (collapsed arrays, \u escapes), which would bury the one line that actually changed.
failure() does not fire for a cancellation, which leaves the draft in exactly the state a failure does — and the asset check takes the newest draft for the tag, so the next run would compare against the stale one.
The Windows probe built cursor-on-event.exe-windows-amd64; the published asset is cursor-on-event-windows-amd64.exe. Every Windows check would have failed on the first release that had Windows binaries to check. Caught by the skip retiring itself: v0.1.26 shipped with Windows assets, the check went live against a real release, and the names did not match. All seven bootstraps now resolve all six platforms in v0.1.26, and expected-artifacts.sh matches its 24 published binaries exactly.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high 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 59847a0. Configure here.
| with: | ||
| # The commit main pointed at when Release was pressed. Pinned rather | ||
| # than re-resolved, so plan and release cannot see different trees. | ||
| ref: ${{ github.sha }} |
There was a problem hiding this comment.
Re-run cannot recover a release
High Severity
Plan, release, and verify all check out the frozen dispatch SHA, so GitHub Re-run never sees a main that moved or already carries the bump. The planner then treats it as a fresh bump and the later push is a non-fast-forward. The step error and DEVELOPMENT.md still say re-running is enough, which leaves main on an unreleased version until someone starts a new dispatch.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 59847a0. Configure here.


Supersedes #223. Same goal, different shape — the shape addresses the review objection to that PR rather than arguing with it.
How you release after this
Actions → Release. Pick
patch(default),minorormajor. That's it — no prepare step, no bump PR, no merge to wait for.dry_runbuilds and checks and publishes nothing;versionsets an exact version instead of a bump.scripts/release.shis deleted. It was macOS-only (sed -i ''), it leftmainprotected so you still had to open a PR and tag by hand, and it was the only thing that knew the PowerShell bootstraps pin a version too.The window is gone
Everything expensive happens while
mainstill points at the old version:Before: unbounded — a human had to remember to tag — then 57 seconds measured when a merge triggered the build. Now two API calls.
Steps 1–5 publish nothing, and a failure in any of them deletes the draft.
Why an App, and what was tested
Pushing to
mainneeds an identity the ruleset lets through. I assumed GitHub Actions' own token could be granted this. It can't:Same on a personal repo and inside
dash0hq. So the Dash0 Release Bot App is named in the bypass list, and used for one command — everything else keepsGITHUB_TOKEN.GITHUB_TOKENpush, no bypassGH013, changes must be made through a pull requestOrganizationAdminas bypass actorGITHUB_TOKENpush, with that bypassPUSH_ACCEPTED— commit6462ab2onmainThe credential is bound to a ref, not to the repo
workflow_dispatchruns the workflow file from the branch it is dispatched from. So any guard inside the tree is one an attacker also controls: push a branch that deletes theREF_NAMEcheck, dispatch it, and a repo secret hands you a token that bypasses branch protection.The App credentials therefore live in a GitHub Environment restricted to
main, not in repo secrets. Measured both ways on a probe branch, with the in-tree guard removed exactly as an attacker would:A job declaring the environment on any other ref never starts; a job that drops the declaration finds nothing to read.
This is why there is no
devchannel. Cutting a prerelease from a feature branch and gating the credential by branch are mutually exclusive without splitting the job graph. Adding it back means moving the push into its own job, which is a change worth making on its own terms.Concurrency and recovery
Someone merges mid-release → step 5 is a plain
git push, so it's a compare-and-swap: rejected ifmainmoved. Nothing was published, the draft is deleted, the run stops. There is no way to publish binaries that don't matchmain, because reaching step 6 requires having won.A run fails after step 5 →
mainnames a version with no release. Dispatch again: the planner seesmainalready carrying an unreleased bump and finishes it rather than bumping past it, which would skip a version permanently.Nothing counted, everything named
Two checks used to be counts, and both were already wrong:
[ "$N" -eq 16 ]— Windows took the build to 24 artifacts. A count also can't say which target went missing.scripts/expected-artifacts.shderives every name from.goreleaser.yaml, and the workflow diffsdist/against it.workflow_callinput. An empty one skipped the check, the bootstrap fell back to the version pinned in the checkout — the old one — and the job reported the new release verified. It's folded intorelease.ymlnow, so the version comes from the plan and cannot arrive empty. The match is exact, too: as a substring,0.1.2was satisfied byon-event-0.1.25-linux-amd64.Testing
The workflow can't be dispatched from a PR, so the branching lives in
scripts/release-plan.shandscripts/version.sh, with 24 contracts intest/contracts/release.sh:The last one runs the real
version.sh setagainst a copy of the tree. Verified failing when either sed stops matching — which is how the PowerShell pins would have drifted, sincerelease.shwas the only place that knew about them.