From 3cadcabe11126632dc3fa8f293395abde42e23ca Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 20 Sep 2026 14:01:15 -0700 Subject: [PATCH] fix(release): keep GitHub releases draft until all assets exist (#21835) electron-builder --publish always was creating a public GitHub release as soon as the first platform uploaded, so /releases/latest could serve a missing Windows exe. Keep the main-repo publisher on draft, pin draft creation to the tag commit, re-draft immediately if anything flips public, and refuse mac publish after the parent cut is cancelled. --- .github/workflows/release-cut.yml | 38 ++--- .github/workflows/release-mac-build.yml | 38 ++--- config/electron-builder.config.cjs | 6 +- .../assert-github-release-is-draft.mjs | 116 +++++++++++++++ .../assert-github-release-is-draft.test.mjs | 139 ++++++++++++++++++ config/scripts/create-draft-release.mjs | 48 ++++-- config/scripts/create-draft-release.test.mjs | 34 ++++- ...ectron-builder-mac-channel-config.test.mjs | 2 +- .../verify-dev-channel-packaging.test.mjs | 2 +- 9 files changed, 360 insertions(+), 63 deletions(-) create mode 100644 config/scripts/assert-github-release-is-draft.mjs create mode 100644 config/scripts/assert-github-release-is-draft.test.mjs diff --git a/.github/workflows/release-cut.yml b/.github/workflows/release-cut.yml index cd940482917..7280a027b0e 100644 --- a/.github/workflows/release-cut.yml +++ b/.github/workflows/release-cut.yml @@ -107,6 +107,10 @@ jobs: with: ref: ${{ github.event_name == 'schedule' && 'main' || inputs.ref }} fetch-depth: 0 + # Why: version math recovers unpublished tags; checkout's default + # fetch-tags:false hides them, so a patch cut recreates vX.Y.Z and + # `git push` overwrites the existing tag. + fetch-tags: true - name: Setup Node.js uses: actions/setup-node@v6 @@ -2193,36 +2197,18 @@ jobs: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - name: Verify release remains draft after artifact upload - # Why: the build matrix must never be the actor that exposes a partial - # release. If an uploader or GitHub transition flips draft early, fail - # this platform leg and leave the diagnostic monitor artifact behind. - shell: bash + # Why: electron-builder `--publish always` can create a public release + # as soon as this platform uploads. Re-draft immediately, then fail, so + # /releases/latest never keeps serving a missing Windows exe. env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} TAG: ${{ needs.cut.outputs.tag }} - run: | - set -euo pipefail - releases_json="$(gh api "repos/$GITHUB_REPOSITORY/releases?per_page=100")" - # Why: release upload must validate the draft before it is publicly visible. - draft="$(jq -e -r --arg tag "$TAG" ' - map(select(.tag_name == $tag)) - | if length == 1 and (.[0].draft | type) == "boolean" then (.[0].draft | tostring) else empty end - ' <<<"$releases_json")" || { - echo "::error::Release $TAG was not found in the draft-aware releases list, or its draft state was missing." - exit 1 - } - if [[ "$draft" != "true" ]]; then - echo "::error::Release $TAG was published during the ${{ matrix.platform }} artifact upload." - exit 1 - fi + run: node config/scripts/assert-github-release-is-draft.mjs "$TAG" - # Why post-publish for Linux: electron-builder packs and uploads in a - # single `--publish always` invocation, so there is no cheap insertion - # point between pack and upload without splitting those steps. Running - # verify last still blocks the bad release: the binary is uploaded to the - # draft, but a failed matrix job blocks `publish-release` from flipping - # the release from draft → published, so users never see it. A human then - # deletes the draft and re-cuts. + # Why post-pack for Linux: electron-builder packs and uploads in one + # `--publish always` invocation. The previous step re-drafts if that + # upload flipped the GitHub release public; this telemetry check still + # blocks `publish-release` from undrafting a bad binary. # # Why this guards against: a misconfigured CI run where # `ORCA_POSTHOG_WRITE_KEY` is unset or the tag fails to classify diff --git a/.github/workflows/release-mac-build.yml b/.github/workflows/release-mac-build.yml index 45193dfe1ae..002622b7154 100644 --- a/.github/workflows/release-mac-build.yml +++ b/.github/workflows/release-mac-build.yml @@ -142,6 +142,21 @@ jobs: # Kill only its child and require both PTY and watch recovery before packaging. node config/scripts/relay-watcher-fault-harness.mjs + - name: Abort if the parent release-cut run was cancelled + # Why: this workflow is dispatched separately, so cancelling release-cut + # does not stop mac `--publish always`. A cancelled parent left v1.4.206 + # public with only a partial mac upload. + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PARENT_RUN: ${{ inputs.release_run_id }} + run: | + set -euo pipefail + conclusion="$(gh api "repos/$GITHUB_REPOSITORY/actions/runs/$PARENT_RUN" --jq '.conclusion // empty')" + if [[ "$conclusion" == "cancelled" || "$conclusion" == "failure" || "$conclusion" == "timed_out" ]]; then + echo "::error::Parent release-cut run $PARENT_RUN is $conclusion; refusing to publish mac artifacts." + exit 1 + fi + - name: Publish release artifacts (macOS) uses: nick-fields/retry@v4 with: @@ -158,28 +173,13 @@ jobs: APPLE_TEAM_ID: ${{ secrets.APPLE_TEAM_ID }} - name: Verify release remains draft after artifact upload - # Why: the macOS build must never be the actor that exposes a partial - # release. If an uploader or GitHub transition flips draft early, fail - # this job so release-cut never publishes the release. - shell: bash + # Why: re-draft immediately if electron-builder flipped the GitHub + # release public, then fail. Checking without restoring leaves + # /releases/latest serving a missing Windows exe. env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} TAG: ${{ inputs.tag }} - run: | - set -euo pipefail - releases_json="$(gh api "repos/$GITHUB_REPOSITORY/releases?per_page=100")" - # Why: release upload must validate the draft before it is publicly visible. - draft="$(jq -e -r --arg tag "$TAG" ' - map(select(.tag_name == $tag)) - | if length == 1 and (.[0].draft | type) == "boolean" then (.[0].draft | tostring) else empty end - ' <<<"$releases_json")" || { - echo "::error::Release $TAG was not found in the draft-aware releases list, or its draft state was missing." - exit 1 - } - if [[ "$draft" != "true" ]]; then - echo "::error::Release $TAG was published during the mac artifact upload." - exit 1 - fi + run: node config/scripts/assert-github-release-is-draft.mjs "$TAG" # Why post-publish for macOS: electron-builder packs and uploads in a # single `--publish always` invocation, so there is no cheap insertion diff --git a/config/electron-builder.config.cjs b/config/electron-builder.config.cjs index 72efa691dbf..60091ed1f93 100644 --- a/config/electron-builder.config.cjs +++ b/config/electron-builder.config.cjs @@ -665,7 +665,11 @@ module.exports = { provider: 'github', owner: 'stablyai', repo: devChannelRepo ?? 'orca', - releaseType: devChannelRepo ? 'prerelease' : 'release' + // Why draft on the main repo: `--publish always` otherwise creates a + // public GitHub release as soon as the first platform uploads, and + // /releases/latest serves a missing Windows exe. release-cut undrafts + // only after every required asset exists. + releaseType: devChannelRepo ? 'prerelease' : 'draft' } } diff --git a/config/scripts/assert-github-release-is-draft.mjs b/config/scripts/assert-github-release-is-draft.mjs new file mode 100644 index 00000000000..c60722452bb --- /dev/null +++ b/config/scripts/assert-github-release-is-draft.mjs @@ -0,0 +1,116 @@ +#!/usr/bin/env node + +import { pathToFileURL } from 'node:url' + +const API_VERSION = '2022-11-28' + +function githubHeaders(token) { + return { + Accept: 'application/vnd.github+json', + Authorization: `Bearer ${token}`, + 'X-GitHub-Api-Version': API_VERSION + } +} + +async function githubJson(fetchImpl, url, token, options = {}) { + const res = await fetchImpl(url, { + ...options, + headers: { + ...githubHeaders(token), + ...options.headers + } + }) + if (!res.ok) { + const body = await res.text().catch(() => '') + throw new Error(`GitHub request failed ${res.status} ${res.statusText}: ${body.slice(0, 300)}`) + } + return res.json() +} + +export function matchingDesktopReleases(releases, tag) { + const version = tag.startsWith('v') ? tag.slice(1) : tag + return (releases ?? []).filter((release) => { + const tagName = release?.tag_name + const name = release?.name + return tagName === tag || tagName === version || name === tag || name === version + }) +} + +export async function restorePublishedDesktopReleasesToDraft({ + repo, + tag, + token, + fetchImpl = fetch, + log = console.log +}) { + if (!repo) { + throw new Error('repo is required') + } + if (!tag) { + throw new Error('tag is required') + } + if (!token) { + throw new Error('token is required') + } + + const releases = await githubJson( + fetchImpl, + `https://api.github.com/repos/${repo}/releases?per_page=100`, + token + ) + if (!Array.isArray(releases)) { + throw new Error(`GitHub releases response for ${repo} was not an array`) + } + + const matches = matchingDesktopReleases(releases, tag) + if (matches.length === 0) { + throw new Error(`No GitHub release named ${tag} was found after artifact upload`) + } + + const restored = [] + for (const release of matches) { + if (release?.draft === true) { + continue + } + if (!Number.isInteger(release.id)) { + throw new Error(`Release ${tag} is missing a GitHub release id`) + } + const patched = await githubJson( + fetchImpl, + `https://api.github.com/repos/${repo}/releases/${release.id}`, + token, + { + method: 'PATCH', + body: JSON.stringify({ draft: true, make_latest: 'false' }) + } + ) + log(`Restored GitHub release ${release.id} (${release.tag_name}) to draft.`) + restored.push(patched) + } + return restored +} + +async function main() { + const tag = process.argv[2] + const token = process.env.GH_TOKEN || process.env.GITHUB_TOKEN + const repo = process.env.GITHUB_REPOSITORY || 'stablyai/orca' + const restored = await restorePublishedDesktopReleasesToDraft({ + repo, + tag, + token, + log: (message) => console.error(message) + }) + if (restored.length > 0) { + console.error( + `::error::Release ${tag} was published during artifact upload. Restored ${restored.length} release(s) to draft so /releases/latest does not serve partial assets.` + ) + process.exit(1) + } +} + +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + main().catch((error) => { + console.error(error.message) + process.exit(1) + }) +} diff --git a/config/scripts/assert-github-release-is-draft.test.mjs b/config/scripts/assert-github-release-is-draft.test.mjs new file mode 100644 index 00000000000..b549d25b59d --- /dev/null +++ b/config/scripts/assert-github-release-is-draft.test.mjs @@ -0,0 +1,139 @@ +import { readFileSync } from 'node:fs' +import { createRequire } from 'node:module' +import { join } from 'node:path' +import { describe, expect, it, vi } from 'vitest' +import { parse } from 'yaml' +import { + matchingDesktopReleases, + restorePublishedDesktopReleasesToDraft +} from './assert-github-release-is-draft.mjs' + +const require = createRequire(import.meta.url) +const repoRoot = join(import.meta.dirname, '../..') + +function jsonResponse(body, init = {}) { + return { + ok: init.ok ?? true, + status: init.status ?? 200, + statusText: init.statusText ?? 'OK', + json: vi.fn(async () => body), + text: vi.fn(async () => JSON.stringify(body)) + } +} + +describe('matchingDesktopReleases', () => { + it('matches tagged, untagged-name, and version-name releases', () => { + const releases = [ + { id: 1, tag_name: 'v1.4.206', name: 'v1.4.206', draft: true }, + { id: 2, tag_name: 'untagged-abc', name: '1.4.206', draft: false }, + { id: 3, tag_name: 'v1.4.205', name: 'v1.4.205', draft: false } + ] + + expect(matchingDesktopReleases(releases, 'v1.4.206').map((release) => release.id)).toEqual([ + 1, 2 + ]) + }) +}) + +describe('restorePublishedDesktopReleasesToDraft', () => { + it('leaves drafts alone', async () => { + const fetchImpl = vi + .fn() + .mockResolvedValueOnce( + jsonResponse([{ id: 1, tag_name: 'v1.4.206', name: 'v1.4.206', draft: true }]) + ) + + await expect( + restorePublishedDesktopReleasesToDraft({ + repo: 'stablyai/orca', + tag: 'v1.4.206', + token: 'token', + fetchImpl, + log: vi.fn() + }) + ).resolves.toEqual([]) + expect(fetchImpl).toHaveBeenCalledTimes(1) + }) + + it('re-drafts a published match immediately', async () => { + const fetchImpl = vi + .fn() + .mockResolvedValueOnce( + jsonResponse([{ id: 9, tag_name: 'v1.4.206', name: '1.4.206', draft: false }]) + ) + .mockResolvedValueOnce(jsonResponse({ id: 9, tag_name: 'v1.4.206', draft: true })) + + const log = vi.fn() + await expect( + restorePublishedDesktopReleasesToDraft({ + repo: 'stablyai/orca', + tag: 'v1.4.206', + token: 'token', + fetchImpl, + log + }) + ).resolves.toEqual([{ id: 9, tag_name: 'v1.4.206', draft: true }]) + + expect(fetchImpl).toHaveBeenNthCalledWith( + 2, + 'https://api.github.com/repos/stablyai/orca/releases/9', + expect.objectContaining({ + method: 'PATCH', + body: JSON.stringify({ draft: true, make_latest: 'false' }) + }) + ) + expect(log).toHaveBeenCalledWith('Restored GitHub release 9 (v1.4.206) to draft.') + }) + + it('fails closed when no matching release exists', async () => { + const fetchImpl = vi.fn().mockResolvedValueOnce(jsonResponse([])) + + await expect( + restorePublishedDesktopReleasesToDraft({ + repo: 'stablyai/orca', + tag: 'v1.4.206', + token: 'token', + fetchImpl + }) + ).rejects.toThrow('No GitHub release named v1.4.206 was found after artifact upload') + }) +}) + +describe('release draft workflow contract', () => { + it('keeps GitHub releases draft until publish-release undrafts complete assets', () => { + const releaseWorkflow = parse( + readFileSync(join(repoRoot, '.github/workflows/release-cut.yml'), 'utf8') + ) + const macWorkflow = parse( + readFileSync(join(repoRoot, '.github/workflows/release-mac-build.yml'), 'utf8') + ) + const electronBuilderConfig = require('../electron-builder.config.cjs') + const cutCheckout = releaseWorkflow.jobs.cut.steps.find((step) => step.name === 'Checkout ref') + const linuxDraftStep = releaseWorkflow.jobs.build.steps.find( + (step) => step.name === 'Verify release remains draft after artifact upload' + ) + const publishRelease = releaseWorkflow.jobs['publish-release'].steps.find( + (step) => step.name === 'Publish release' + ) + const macSteps = macWorkflow.jobs['build-mac'].steps + const abortParentStep = macSteps.find( + (step) => step.name === 'Abort if the parent release-cut run was cancelled' + ) + const macPublishStep = macSteps.find( + (step) => step.name === 'Publish release artifacts (macOS)' + ) + const macDraftStep = macSteps.find( + (step) => step.name === 'Verify release remains draft after artifact upload' + ) + + expect(electronBuilderConfig.publish.releaseType).toBe('draft') + expect(cutCheckout.with['fetch-tags']).toBe(true) + expect(linuxDraftStep.run).toContain('assert-github-release-is-draft.mjs') + expect(publishRelease.run).toContain('gh release edit') + expect(publishRelease.run).toContain('--draft=false') + expect(macSteps.indexOf(abortParentStep)).toBeLessThan(macSteps.indexOf(macPublishStep)) + expect(abortParentStep.env.PARENT_RUN).toBe('${{ inputs.release_run_id }}') + expect(abortParentStep.run).toContain('refusing to publish mac artifacts') + expect(macDraftStep.run).toContain('assert-github-release-is-draft.mjs') + }) +}) diff --git a/config/scripts/create-draft-release.mjs b/config/scripts/create-draft-release.mjs index 1732e9a1e8a..a25bced199b 100644 --- a/config/scripts/create-draft-release.mjs +++ b/config/scripts/create-draft-release.mjs @@ -1,5 +1,6 @@ #!/usr/bin/env node +import { execFileSync } from 'node:child_process' import { pathToFileURL } from 'node:url' const API_VERSION = '2022-11-28' @@ -115,6 +116,7 @@ export async function createDraftRelease({ repo, tag, token, + targetCommitish, fetchImpl = fetch, log = console.log }) { @@ -127,6 +129,9 @@ export async function createDraftRelease({ if (!token) { throw new Error('token is required') } + if (!targetCommitish) { + throw new Error('targetCommitish is required') + } const releases = await fetchRepoReleases(repo, token, fetchImpl) const existingRelease = releases.find((release) => release?.tag_name === tag) @@ -221,19 +226,33 @@ export async function createDraftRelease({ return } } else { - // Why: GitHub's generated release notes can exceed the release body API - // limit, so create with a bounded body. Omit target_commitish because the - // release-cut tag already exists and GitHub rejects the tag name there. - await githubJson(fetchImpl, `https://api.github.com/repos/${repo}/releases`, token, { - method: 'POST', - body: JSON.stringify({ - tag_name: tag, - name, - body, - draft: true, - prerelease - }) - }) + // Why target_commitish is the tag commit, not omitted: GitHub defaults it + // to the repo default branch. A release-cut tag is a detached bump commit, + // so that default creates an untagged draft. electron-builder then misses + // it by tag name and `--publish always` opens a public release with the + // first platform's assets, which /releases/latest serves without the exe. + const createdRelease = await githubJson( + fetchImpl, + `https://api.github.com/repos/${repo}/releases`, + token, + { + method: 'POST', + body: JSON.stringify({ + tag_name: tag, + target_commitish: targetCommitish, + name, + body, + draft: true, + prerelease, + make_latest: 'false' + }) + } + ) + if (createdRelease?.draft !== true || createdRelease?.tag_name !== tag) { + throw new Error( + `GitHub created ${createdRelease?.draft ? 'draft' : 'published'} release ${createdRelease?.tag_name ?? ''} instead of draft ${tag}` + ) + } } if (generatedBody.length !== body.length) { @@ -251,7 +270,8 @@ async function main() { const tag = process.argv[2] const token = process.env.GH_TOKEN || process.env.GITHUB_TOKEN const repo = process.env.GITHUB_REPOSITORY || 'stablyai/orca' - await createDraftRelease({ repo, tag, token }) + const targetCommitish = execFileSync('git', ['rev-parse', 'HEAD'], { encoding: 'utf8' }).trim() + await createDraftRelease({ repo, tag, token, targetCommitish }) } if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { diff --git a/config/scripts/create-draft-release.test.mjs b/config/scripts/create-draft-release.test.mjs index 911ac00be63..8dc36369b32 100644 --- a/config/scripts/create-draft-release.test.mjs +++ b/config/scripts/create-draft-release.test.mjs @@ -140,6 +140,7 @@ describe('createDraftRelease', () => { repo: 'stablyai/orca', tag: 'v1.4.36', token: 'token', + targetCommitish: 'abc123', fetchImpl, log: vi.fn() }) @@ -173,9 +174,11 @@ describe('createDraftRelease', () => { const createBody = JSON.parse(fetchImpl.mock.calls[2][1].body) expect(createBody).toMatchObject({ tag_name: 'v1.4.36', + target_commitish: 'abc123', name: 'v1.4.36', draft: true, - prerelease: false + prerelease: false, + make_latest: 'false' }) expect(createBody.body).toHaveLength(120_000) expect(createBody.body).toContain('Release notes were truncated') @@ -192,6 +195,7 @@ describe('createDraftRelease', () => { repo: 'stablyai/orca', tag: 'v1.4.36-rc.1', token: 'token', + targetCommitish: 'abc123', fetchImpl, log: vi.fn() }) @@ -214,6 +218,7 @@ describe('createDraftRelease', () => { repo: 'stablyai/orca', tag: 'v1.4.36', token: 'token', + targetCommitish: 'abc123', fetchImpl, log: vi.fn() }) @@ -243,6 +248,7 @@ describe('createDraftRelease', () => { repo: 'stablyai/orca', tag: 'v1.4.36', token: 'token', + targetCommitish: 'abc123', fetchImpl, log: vi.fn() }) @@ -272,6 +278,7 @@ describe('createDraftRelease', () => { repo: 'stablyai/orca', tag: 'v1.4.36', token: 'token', + targetCommitish: 'abc123', fetchImpl, log }) @@ -304,6 +311,7 @@ describe('createDraftRelease', () => { repo: 'stablyai/orca', tag: 'v1.4.36', token: 'token', + targetCommitish: 'abc123', fetchImpl, log }) @@ -319,6 +327,7 @@ describe('createDraftRelease', () => { repo: 'stablyai/orca', tag: 'v1.4.36', token: 'token', + targetCommitish: 'abc123', fetchImpl, log: vi.fn() }) @@ -337,6 +346,7 @@ describe('createDraftRelease', () => { repo: 'stablyai/orca', tag: 'v1.4.36', token: 'token', + targetCommitish: 'abc123', fetchImpl, log: vi.fn() }) @@ -359,6 +369,7 @@ describe('createDraftRelease', () => { repo: 'stablyai/orca', tag: 'v1.4.36', token: 'token', + targetCommitish: 'abc123', fetchImpl, log: vi.fn() }) @@ -376,4 +387,25 @@ describe('createDraftRelease', () => { const generateNotesBody = JSON.parse(fetchImpl.mock.calls[2][1].body) expect(generateNotesBody.previous_tag_name).toBe('v1.4.35') }) + + it('refuses an untagged GitHub draft so electron-builder cannot publish latest', async () => { + const fetchImpl = vi + .fn() + .mockResolvedValueOnce(jsonResponse([])) + .mockResolvedValueOnce(jsonResponse({ name: 'v1.4.36', body: 'notes' })) + .mockResolvedValueOnce( + jsonResponse({ tag_name: 'untagged-abc', name: 'v1.4.36', draft: true }) + ) + + await expect( + createDraftRelease({ + repo: 'stablyai/orca', + tag: 'v1.4.36', + token: 'token', + targetCommitish: 'abc123', + fetchImpl, + log: vi.fn() + }) + ).rejects.toThrow('GitHub created draft release untagged-abc instead of draft v1.4.36') + }) }) diff --git a/config/scripts/electron-builder-mac-channel-config.test.mjs b/config/scripts/electron-builder-mac-channel-config.test.mjs index dbd5a1170a1..05572f3497a 100644 --- a/config/scripts/electron-builder-mac-channel-config.test.mjs +++ b/config/scripts/electron-builder-mac-channel-config.test.mjs @@ -80,7 +80,7 @@ describe('electron-builder mac channel config', () => { }) expect(electronBuilderConfig.publish).toMatchObject({ repo: 'orca', - releaseType: 'release' + releaseType: 'draft' }) }) diff --git a/config/scripts/verify-dev-channel-packaging.test.mjs b/config/scripts/verify-dev-channel-packaging.test.mjs index 8e5a00f48e1..dfde5d461d2 100644 --- a/config/scripts/verify-dev-channel-packaging.test.mjs +++ b/config/scripts/verify-dev-channel-packaging.test.mjs @@ -40,7 +40,7 @@ describe('electron-builder dev-channel identity', () => { expect(config.win.signtoolOptions.publisherName).toBe('SignPath Foundation') expect(config.win.verifyUpdateCodeSignature).toBeUndefined() expect(config.publish.repo).toBe('orca') - expect(config.publish.releaseType).toBe('release') + expect(config.publish.releaseType).toBe('draft') }) // The whole point of the change: an unsigned build that advertised a