Skip to content

fix(release): draft-then-promote releases; un-pin npm past sigstore bug - #277

Merged
willem445 merged 1 commit into
mainfrom
fix/275-draft-then-promote
Jul 14, 2026
Merged

willem445 merged 1 commit into
mainfrom
fix/275-draft-then-promote

Conversation

@willem445

Copy link
Copy Markdown
Owner

Summary

Two related hardening fixes to release.yml, both flagged from PR #272's review.

#275 — draft-then-promote. build is a 4-leg matrix uploading assets to one GitHub release. Previously that release was created public/non-draft (releaseDraft: false) the moment the first leg ran, so a partial failure (one leg fails after others already uploaded) left a public latest release missing assets — install.sh/install.ps1 resolve latest and hit 404s on the missing files.

Fix: build now creates the release as a draft (releaseDraft: true, tauri-action's native support). A new promote job (needs: build) flips it public with gh release edit --draft=false — and since fail-fast: false means a single failed leg fails the build job as a whole, promote is skipped whenever any leg fails, leaving the release stuck in draft rather than public-and-incomplete. latest can only ever resolve to a complete release.

publish-npm now depends on promote, not just build. Reasoning: GitHub does not serve release assets off a draft release to anonymous downloads, and the npm launcher (npx loomux-desktop) resolves those assets at runtime, not at publish time — so if publish-npm only waited on build, a user could npx loomux-desktop between npm-publish and the (manual/delayed) promotion and get a 404. Waiting on promote transitively waits on build too, so #221's ordering guarantee is preserved.

#186 — un-pin npm past the sigstore bug. npm/cli#9722 (npm 12.0.0 shipped without its bundled sigstore module, breaking provenance publish with MODULE_NOT_FOUND) is closed, but per the issue's standing instruction I verified the fix actually ships in a reachable version rather than trusting issue state alone:

  • Fixed by npm/cli#9740 ("correct bundled sigstore from dev dependency conflict"), merged 2026-07-10.
  • Released in npm 12.0.1 the same day.
  • Verified directly: downloaded npm-12.0.1.tgz from the registry and confirmed node_modules/sigstore is present (5 files, matching the working 11.18.0 bundle) — not just reading the changelog.

Moved the pin from npm@^11 to npm@^12.0.1 — excludes the broken 12.0.0 specifically while tracking future 12.x patches, same style as the old range tracking 11.x patches.

Out of scope / follow-up

  • If build fails and promote is skipped, the draft release is left behind for manual cleanup (or a retry) — no automated draft-deletion was requested by either issue, noting it here rather than silently expanding scope.
  • OIDC trusted publishing (id-token: write, no NPM_TOKEN) is untouched.

Test plan

  • actionlint (v1.7.7, prebuilt Windows binary) — clean, no findings.
  • Hand-verified the new/changed run: shell blocks under bash -e semantics (actionlint skips shellcheck without it on PATH): both are single-line commands, no multi-line/heredoc pitfalls. Fixed one issue in review: the promote step originally interpolated ${{ github.ref_name }}/${{ github.repository }} directly into run: (classic Actions script-injection pattern for untrusted-ish tag names) — switched to the default env vars $GITHUB_REF_NAME/$GITHUB_REPOSITORY, matching the existing safe convention already used for VERSION in publish-npm.
  • Real release run (draft creation, matrix upload, promote, npm publish end-to-end) — needs an actual tag push; left for the next release cut per repo convention (agents don't spawn real publishes).

Closes #275, Closes #186

…ug (#275, #186)

Create the tauri-action release as a draft; add a promote job (needs:
build) that flips it public only once every matrix leg has uploaded,
so `latest` can never resolve to a partial release. publish-npm now
waits on promote (not just build), since GitHub won't serve assets off
a draft release to anonymous downloads and the npm launcher fetches
them at runtime.

Un-pin npm past npm@^11: npm/cli#9722 (missing sigstore, breaking
provenance publish) was fixed by npm/cli#9740 and shipped in npm
12.0.1 (2026-07-10) — verified by extracting the published tarball and
confirming node_modules/sigstore is bundled again. ^12.0.1 tracks
future 12.x patches while excluding the broken 12.0.0.

@willem445 willem445 left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: Approve — 2 non-blocking findings. The happy path and the common re-run-failed path are correct; the injection hardening is right; every npm/sigstore claim in the PR body checks out against primary sources. The two findings are rare, recoverable, and largely pre-existing upstream behavior — not release-blockers — but they mean the PR's "latest can only ever resolve to a complete release" guarantee is "almost always," not "always," so I'm documenting them for the human before the cut.

I verified the mechanics against source, not just the PR narrative:

  • tauri-action create-release.ts — with releaseDraft: true it does not use getReleaseByTag (which 404s on drafts). It pages listReleases (drafts included) and matches by tag_name. So the four matrix legs do converge on one draft — provided the first leg's draft exists before later legs run their lookup. The check-then-create is not atomic.
  • tauri-action upload-release-assets.ts — deletes any same-named existing asset, then re-uploads. So re-running one leg against an existing draft is idempotent (no double-upload/conflict).
  • gh shared/fetch.go FetchRelease — runs the published lookup and a GraphQL draft-by-pending-tag lookup concurrently and takes whichever resolves. So gh release edit "$GITHUB_REF_NAME" --draft=false does find the draft tauri-action created (it sets tag_name on the draft). Promote's targeting is sound for the single-draft case.

What's correct (checked, not assumed)

#186 / npm pin — fully verified independently:

  • npm/cli#9740 ("correct bundled sigstore from dev dependency conflict") is merged 2026-07-10.
  • npm 12.0.1 published 2026-07-10, and is the current latest dist-tag.
  • Pulled npm-12.0.1.tgz from the registry: node_modules/sigstore/ is present (5 files) — matches the PR body's claim exactly.
  • npm install -g npm@^12.0.1 resolves to 12.0.1+, excludes the broken 12.0.0, and overrides whatever npm Node 22 bundles — so the pin is effective at the point that matters (the provenance npm publish). Good.

Injection hardening (item 5) — correct everywhere. The only run: block touching the tag is promote, and it uses $GITHUB_REF_NAME/$GITHUB_REPOSITORY (quoted), not ${{ }}. The remaining ${{ github.ref_name }} uses are in the build job's with: inputs (tagName, releaseName) — action inputs, not shell — so no substitution-into-shell remains. publish-npm already used $GITHUB_REF_NAME.

Permissions survive the re-wire. promote declares contents: write (needed for gh release edit; no checkout needed — gh is preinstalled, GH_TOKEN set). publish-npm keeps id-token: write + contents: read, so OIDC trusted publishing is untouched. needs: promote transitively preserves the #221 ordering.

Shell under bash -e (item 6, shellcheck caveat). Both ubuntu jobs run bash with set -eo pipefail. promote is a single quoted command — a non-zero exit (e.g. draft not found) correctly fails the job loudly. publish-npm's npm view guard sits in an if condition, so -e doesn't fire on the not-yet-published case — the idempotence guard from #272 is intact. GITHUB_REF_NAME is always set on a tag push. No unset-var or unpropagated-failure traps. (Note: I could not run actionlint locally — not installed in this worktree — so this is a hand-check; the PR's actionlint-clean claim is plausible given the YAML is well-formed.)

Full release walk-through

Happy path (git push of a v* tag): trigger → 4 build legs. Legs finish at very different wall-clock times (different OSes, staggered Rust compiles), so the first to reach tauri-action creates the single draft; the later three find it via listReleases and upload into it. All 4 green → build job green → promote runs → gh release edit <tag> --draft=false flips the one draft public → publish-npm runs, bumps version, guard sees it unpublished, npm publish. latest first resolves only after promote. ✅ Correct.

One leg fails (the common failure): fail-fast:false → other 3 upload, but build is red → promote and publish-npm are skipped → release stays a draft → latest never resolves to it. ✅ Exactly the intended safe direction, and re-runnable (see below).

Re-run failed jobs (common retry) — handled well: only the failed leg + skipped promote/publish-npm re-run. The failed leg finds the still-draft release and uploads (delete-then-re-upload → idempotent even if it had partially uploaded before failing). promote then flips the now-complete draft public; npm publishes. ✅

If promote itself fails after all builds succeed: release stays draft, latest unaffected — safe. Detectable (red promote job) and re-runnable (re-run just promote; gh release edit --draft=false on an already-correct draft is fine). ✅

promote succeeds / npm fails, then rerun: re-run publish-npm; the npm view guard skips if the version already published, else publishes. Release already public, unaffected. ✅

Finding 1 (non-blocking) — duplicate-draft matrix race weakens the "always complete" guarantee

create-release.ts's list-then-create is not atomic (upstream tauri-action #914, observed ~2–3 per 78 releases). If two legs race before either draft is visible to the other, you get two draft releases for the same tag, each with a subset of assets. All legs still exit 0, so build is green and promote runs. gh's draft lookup (GraphQL release(tagName:)) returns one of them — possibly the incomplete one — and flips that public. Result: the partial-public-latest failure #275 set out to prevent, reachable via the race, plus an orphan draft.

Why non-blocking: (a) it's pre-existing upstream behaviour of the matrix-without-releaseId pattern, neither introduced nor fixed here; (b) staggered build times make it rare in practice; (c) the PR is still a strict improvement over the old public-from-first-leg flow for the dominant single-leg-failure case. But the PR body / code comment claim latest can only ever be complete — that's overstated; please soften the wording. The robust fix (follow-up, not this PR) is tauri-action's own documented pattern: a separate create-release job that creates the draft once and emits releaseId, which the matrix legs consume — that removes the race and makes promote's target unambiguous.

Finding 2 (non-blocking) — "Re-run all jobs" after a successful release spawns a duplicate draft

Confirmed from create-release.ts: with releaseDraft: true, when a leg finds an existing release for the tag that is not a draft (i.e. already promoted to public), it logs "NOT a draft", breaks, treats it as not-found, and creates a fresh draft. So clicking "Re-run all jobs" on an already-published release: all 4 legs re-upload into a brand-new duplicate draft; then promote's gh release edit either no-ops on the existing public release or hits a 422 trying to publish a second release for the same tag (nondeterministic, depending on which concurrent lookup in FetchRelease wins) — leaving a stray fully-populated duplicate draft behind. npm is protected by its idempotence guard.

This is a behavioural regression vs. the old releaseDraft: false flow, where a re-run-all cleanly re-uploaded into the existing public release (idempotent). Non-blocking because it doesn't affect a fresh release cut and the original public release/latest survives (worst case is a red promote + manual draft cleanup), but it contradicts the re-run-safety framing and is worth a one-line caveat in the PR's "Out of scope" list (alongside the existing draft-cleanup note). The releaseId-job refactor in Finding 1 would also fix this.

Bottom line

Ship it for the imminent release — the fresh-tag happy path and the common retry path are correct and the npm fix is real. Just (1) soften the "only ever complete" wording to acknowledge the residual matrix race, and (2) add the re-run-all caveat. Both findings point at the same optional follow-up (separate create-release job emitting releaseId); neither should hold the release.

@willem445
willem445 merged commit 375d933 into main Jul 14, 2026
3 checks passed
@willem445
willem445 deleted the fix/275-draft-then-promote branch July 14, 2026 22:59
willem445 added a commit that referenced this pull request Jul 15, 2026
…#282) (#296)

* fix(release): dedicated create-release job kills duplicate-draft race (#282)

Follow-up from PR #277's review, upgraded from ~3%-edge-case to
observed-on-first-run: v0.9.0's live release run created TWO draft
releases for the same tag (tauri-action's per-leg listReleases-or-
create lookup raced, per upstream tauri-apps/tauri-action#914).
`promote` flipped the one with 5/9 assets public; the other 4 assets
sat stranded on a stray draft, recovered by hand.

Fix shape (from the issue): a dedicated `create-release` job creates
the draft ONCE and emits `release_id` as a job output. Every `build`
matrix leg targets that exact id via tauri-action's `releaseId` input
instead of doing its own lookup, so the race has nothing left to race
on. `create-release` is also idempotent (reuses an existing release
for the tag if one's already there) — addresses the issue's second
residual problem, where "Re-run all jobs" on a partially-failed run
used to spawn a fresh duplicate draft.

`promote` now also verifies the release's own asset count (expects 9,
per the SKILL.md checklist) against `release_id` directly before
flipping it public, and refuses (stays draft) if short — per the
issue's addendum, trusting `needs: build` succeeding was exactly what
let the incomplete v0.9.0 release go public even though `build`
concluded 'success'.

Preserves: draft-then-promote semantics (#275), publish-npm's
dependency on promote (#221 — unchanged), $GITHUB_REF_NAME (not
template substitution) for untrusted-ish values in shell steps, and
the npm-publish idempotence guard (`npm view` check, untouched).

Updated SKILL.md's "watch the workflow" section for the new job and
the new failure mode (promote refusing on a short asset count).

## Validation

Can't run a real release (agents don't spawn releases/tags in this
repo) — validated by:
- `actionlint` v1.7.12 (pinned, checksum-verified binary — same
  tooling as #276, run directly, no dependency on that PR's branch):
  clean, no findings, on the full changed workflow.
- `npm run build` and `cargo check --locked` (isolated
  CARGO_TARGET_DIR): both pass.
- Dry-reasoning through each failure mode (below).

Live validation happens at the next real release cut, as usual for
release.yml changes in this repo.

### Failure-mode reasoning

**Build fails mid-matrix.** `create-release` still succeeds and
creates the draft. `fail-fast: false` lets the other 3 legs finish
uploading their assets to that same draft id. `build` as a whole
reports failure (one leg failed), so `promote` (needs: build) is
skipped — release stays in draft with a partial asset set, exactly
today's draft-then-promote behavior, now additionally protected by
the asset-count check if `promote` were ever run against it by hand.

**Re-run of a failed run.** Two cases:
- "Re-run failed jobs": GitHub Actions reuses the prior attempt's
  outputs for jobs that already succeeded — `create-release` doesn't
  re-execute, its `release_id` output is reused as-is, so failed
  `build` legs retry against the SAME release. No new draft.
- "Re-run all jobs": `create-release` re-executes, but its idempotence
  check (`listReleases` filtered by `tag_name`) finds the release the
  prior attempt already created and reuses its id instead of creating
  a second one. This is the exact regression the issue called out
  ("spawns a NEW duplicate draft on re-run") — closed.

**Promote sees a short asset count.** `needs: build` succeeding is
necessary but not sufficient, per the v0.9.0 incident (build concluded
'success' with assets split across two releases). `promote`'s first
step counts assets on `release_id` directly (not a tag-based lookup —
the same ambiguity that let the incident through undetected) and
`exit 1`s before the publish step if short, leaving the release in
draft. A human then compares against `gh api .../releases` for a
stray duplicate, or re-runs the genuinely-missing leg — documented in
the updated SKILL.md.

* fix(release): add workflow-level concurrency guard for same-tag runs (#282)

Non-blocking review note on PR #296: create-release's idempotence
check (look up a release for this tag, reuse if found) closes the
per-leg race within a single run, but two separate RUNS of this
workflow for the same tag — a tag force-moved and re-pushed while the
first run is still in flight, or a manual re-trigger — could still
both pass that lookup before either has created the release. Same
underlying race (#282), one level up.

Added a workflow-level `concurrency` group keyed on `github.ref`, so
a second run for the same tag queues behind the first instead of
racing it. `cancel-in-progress: false` is deliberate, not the
default-omitted choice: cancelling a release run mid-flight is
exactly the kind of partial state #282's race exploited (half-
uploaded matrix, draft promoted without npm published), so queuing
instead of cancelling guarantees the second run's create-release
lookup only ever sees a fully-formed release or none — never one
still being written to.

The reviewer's pagination note on create-release's listReleases call
needs no change (unbounded release history isn't a near-term concern
at this repo's release cadence, first page covers it).

* fix(release): pin release notes to release_id, never tag lookup (#282)

Human-recorded requirement from the v0.9.0 incident: release notes
must always land on the OFFICIAL release, never a draft/stray.

- SKILL.md step 4 now applies notes with
  `gh api -X PATCH repos/OWNER/REPO/releases/RELEASE_ID -F body=@notes.md`
  against the canonical release_id (findable in create-release's and
  promote's run logs), instead of `gh release edit vX.Y.Z
  --notes-file -`. Tag-based lookup is exactly the ambiguity that let
  v0.9.0's incident happen — two draft releases existed for the same
  tag — so every operation (assets, promotion, and now notes) targets
  release_id, never the tag.
- `promote` gets a new non-fatal step, before it flips the release
  public, that warns (::warning, doesn't exit 1) if the release body
  is still empty — a notes-less publish is now loud in the run log
  instead of silently discoverable after the fact. It's intentionally
  non-blocking: missing notes are a human oversight to flag, not a
  reason to hold back a complete, correctly-counted release.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant