From f5550c4fde24d6525ef2da5974720a9782f7ffab Mon Sep 17 00:00:00 2001 From: Joey Stanford Date: Mon, 10 Aug 2026 16:38:07 -0600 Subject: [PATCH 1/2] fix(ci): make prepare the sole GitHub draft creator Stop electron-builder and Flatpak softprops from POSTing competing drafts: build with --publish never, upload by prepare release_id, wait for the draft on Flatpak, and soft-fail finalize metadata PATCHes that 403 under GITHUB_TOKEN. --- .github/workflows/flatpak.yaml | 20 +- .github/workflows/release.yaml | 21 ++- docs/ci-cd.md | 18 +- docs/release-process.md | 32 ++-- docs/troubleshooting.md | 2 +- scripts/ci-ensure-github-draft-release.mjs | 18 +- .../ci-ensure-github-draft-release.test.mjs | 120 +++++++++++- .../ci-patch-draft-release-schema-note.mjs | 52 ++++-- ...i-patch-draft-release-schema-note.test.mjs | 10 +- scripts/ci-upload-release-assets.mjs | 122 ++++++++++++ scripts/ci-upload-release-assets.test.mjs | 77 ++++++++ scripts/ci-wait-github-draft-release.mjs | 27 +++ scripts/github-release-api.mjs | 174 ++++++++++++++++-- scripts/resolve-release-matrix.mjs | 12 +- 14 files changed, 624 insertions(+), 81 deletions(-) create mode 100644 scripts/ci-upload-release-assets.mjs create mode 100644 scripts/ci-upload-release-assets.test.mjs create mode 100644 scripts/ci-wait-github-draft-release.mjs diff --git a/.github/workflows/flatpak.yaml b/.github/workflows/flatpak.yaml index adba2a02c..a20b7ecf3 100644 --- a/.github/workflows/flatpak.yaml +++ b/.github/workflows/flatpak.yaml @@ -276,18 +276,16 @@ jobs: id: release_tag run: echo "value=v$(node -p "require('./package.json').version")" >> "$GITHUB_OUTPUT" - - name: Ensure draft GitHub release exists + # Never POST /releases here — wait for Electron prepare-github-release draft. + - name: Wait for draft GitHub release + id: draft env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - GITHUB_SHA: ${{ github.sha }} RELEASE_TAG: ${{ steps.release_tag.outputs.value }} - run: node scripts/ci-ensure-github-draft-release.mjs + run: node scripts/ci-wait-github-draft-release.mjs - - name: Attach Flatpak to release - # softprops/action-gh-release v3 (Node 24) - # draft: true — required so we do not auto-publish an existing draft release - # after uploading assets (default behavior when draft is omitted). - uses: softprops/action-gh-release@b4309332981a82ec1c5618f44dd2e27cc8bfbfda - with: - draft: true - files: flatpak-dist/*.flatpak + - name: Attach Flatpak to draft release + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + RELEASE_ID: ${{ steps.draft.outputs.release_id }} + run: node scripts/ci-upload-release-assets.mjs flatpak-dist/*.flatpak diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index e8fd65117..683b4a101 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -62,6 +62,9 @@ jobs: runs-on: ubuntu-latest permissions: contents: write + outputs: + release_tag: ${{ steps.release_tag.outputs.value }} + release_id: ${{ steps.ensure.outputs.release_id }} steps: - name: Checkout code uses: actions/checkout@v6 @@ -72,11 +75,14 @@ jobs: id: release_tag run: echo "value=v$(node -p "require('./package.json').version")" >> "$GITHUB_OUTPUT" + # Sole job allowed to POST /releases (MESH_CLIENT_ALLOW_DRAFT_CREATE=1). - name: Ensure draft GitHub release exists + id: ensure env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} GITHUB_SHA: ${{ github.sha }} RELEASE_TAG: ${{ steps.release_tag.outputs.value }} + MESH_CLIENT_ALLOW_DRAFT_CREATE: '1' run: node scripts/ci-ensure-github-draft-release.mjs # Rebuild schema note from compare-job outputs (not the downloaded artifact) so the @@ -86,6 +92,7 @@ jobs: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} GITHUB_SHA: ${{ github.sha }} RELEASE_TAG: ${{ steps.release_tag.outputs.value }} + RELEASE_ID: ${{ steps.ensure.outputs.release_id }} MESH_CLIENT_SCHEMA_CURR: ${{ needs.schema-release-compare.outputs.curr_schema }} MESH_CLIENT_SCHEMA_PREV: ${{ needs.schema-release-compare.outputs.prev_schema }} MESH_CLIENT_SCHEMA_PREV_TAG: ${{ needs.schema-release-compare.outputs.prev_tag }} @@ -214,10 +221,8 @@ jobs: MESH_CLIENT_BUILD_WORKFLOW: Build/Release Electron App run: node scripts/ci-write-build-info-env.mjs - - name: Build and Publish + - name: Build packages env: - # electron-publish reads GH_TOKEN; GITHUB_TOKEN alone is not used for uploads - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} # ── macOS code signing (only effective on macOS runners) ──────────── # CSC_LINK: base64-encoded .p12 Developer ID Application certificate. # CSC_KEY_PASSWORD: password protecting the .p12 file. @@ -230,9 +235,17 @@ jobs: APPLE_ID: ${{ matrix.os == 'macos-latest' && secrets.APPLE_ID || '' }} APPLE_APP_SPECIFIC_PASSWORD: ${{ matrix.os == 'macos-latest' && secrets.APPLE_APP_SPECIFIC_PASSWORD || '' }} APPLE_TEAM_ID: ${{ matrix.os == 'macos-latest' && secrets.APPLE_TEAM_ID || '' }} - # dist:*:publish scripts run electron-builder with --publish always + # dist:* uses --publish never so electron-builder never POSTs a draft release. run: ${{ matrix.build_script }} + - name: Upload assets to draft GitHub release + shell: bash + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + RELEASE_ID: ${{ needs.prepare-github-release.outputs.release_id }} + # Attach only — never create a release (prepare owns POST /releases). + run: node scripts/ci-upload-release-assets.mjs ${{ matrix.upload_globs }} + # Do not upload raw Mesh-client.app — upload-artifact@v7 dereferences framework # symlinks (~3× Electron Framework size). DMG/ZIP preserve correct layout. - name: Upload macOS Artifact diff --git a/docs/ci-cd.md b/docs/ci-cd.md index 94bb025a2..4d9873c3f 100644 --- a/docs/ci-cd.md +++ b/docs/ci-cd.md @@ -98,15 +98,15 @@ PR review comments come from [CodeRabbit](https://docs.coderabbit.ai/) via [`.co Triggered by pushing a version tag (e.g., `v1.2.3`): 1. **`schema-release-compare`** — first job; compares this SHA’s `CURRENT_SCHEMA_VERSION` to the last **published** GitHub Release, writes the Actions step summary, and uploads a schema readme artifact. Job outputs feed installer notices and the draft release body. -2. **`prepare-github-release`** — creates a single draft GitHub release for the tag (prevents parallel electron-builder jobs from creating duplicate drafts and 404 asset uploads), then prepends the schema compare note to the draft body. On `workflow_dispatch`, the tag is resolved in the workflow from `package.json` and passed as `RELEASE_TAG` (not read inside the release API script — avoids CodeQL `js/file-access-to-http`). The schema note is rebuilt from `schema-release-compare` job outputs (`MESH_CLIENT_SCHEMA_*`), not from a downloaded markdown artifact (same CodeQL rule). +2. **`prepare-github-release`** — **sole** creator of the draft GitHub release for the tag (`MESH_CLIENT_ALLOW_DRAFT_CREATE=1`), exports `release_id`, then prepends the schema compare note (via `RELEASE_ID`, not List Releases). On `workflow_dispatch`, the tag is resolved in the workflow from `package.json` and passed as `RELEASE_TAG` (not read inside the release API script — avoids CodeQL `js/file-access-to-http`). The schema note is rebuilt from `schema-release-compare` job outputs (`MESH_CLIENT_SCHEMA_*`), not from a downloaded markdown artifact (same CodeQL rule). 3. Installs Linux build dependencies (`libudev-dev`, `rpm`, …) on `ubuntu-latest` runners 4. Rebuilds native dependencies (`pnpm run rebuild`) -5. **Stamp CI build info** — `scripts/ci-write-build-info-env.mjs` writes `MESH_CLIENT_BUILD_INFO` (`buildChannel=release` + tag + Actions `runUrl`) into `$GITHUB_ENV` before `dist:*:publish` so support-bundle `manifest.json` and startup logs identify an official release build (see [Build channel stamp](#build-channel-stamp-test-vs-release)). -6. Builds for all three platforms in parallel (or a filtered subset on `workflow_dispatch`): - - `macos-latest` → `pnpm run dist:mac:publish` - - `ubuntu-latest` → `pnpm run dist:linux:publish` - - `windows-latest` → `pnpm run dist:win:publish` -7. Publishes artifacts to GitHub Releases +5. **Stamp CI build info** — `scripts/ci-write-build-info-env.mjs` writes `MESH_CLIENT_BUILD_INFO` (`buildChannel=release` + tag + Actions `runUrl`) into `$GITHUB_ENV` before `dist:*` so support-bundle `manifest.json` and startup logs identify an official release build (see [Build channel stamp](#build-channel-stamp-test-vs-release)). +6. Builds for all three platforms in parallel (or a filtered subset on `workflow_dispatch`) with **`--publish never`**: + - `macos-latest` → `pnpm run dist:mac` + - `ubuntu-latest` → `pnpm run dist:linux` + - `windows-latest` → `pnpm run dist:win` +7. **`ci-upload-release-assets.mjs`** attaches installers / update metadata to the prepare `release_id` (never `POST /releases`). `finalize-github-release` still consolidates if anything external forked drafts. Linux packaging smoke (`verify-linux-packaging.mjs`) asserts `.deb` **Description** metadata is ASCII-only. See [Release Process](release-process.md). @@ -128,7 +128,7 @@ A matrix builds **x86_64** and **aarch64** in parallel. Both use the same privil 4. Smoke-installs the unstamped local bundle; on **dispatch only**, renames to `org.coloradomesh.MeshClient-run{N}.flatpak` 5. Uploads `org.coloradomesh.MeshClient.flatpak-{x86_64,aarch64}.flatpak` artifacts (file basename stamped on test builds) plus per-arch `flatpak-schema-warning-*` -On **version tag pushes**, a `publish` job attaches both **clean-named** bundles to the GitHub Release. aarch64 is the primary ARM Linux install path (release `build.yaml` only produces x86_64 AppImage/deb/rpm). +On **version tag pushes**, a `publish` job waits for the Electron `prepare-github-release` draft (`ci-wait-github-draft-release.mjs`), then attaches both **clean-named** bundles with `ci-upload-release-assets.mjs` (never creates a release). aarch64 is the primary ARM Linux install path (release `build.yaml` only produces x86_64 AppImage/deb/rpm). `flatpak/generated-sources.json` is generated automatically in CI by `flatpak-node-generator` before each build — it does not need to be committed to the repo. For local builds, generate it manually; see [development-environment.md](development-environment.md) for steps. If submitting to Flathub's dedicated submission repo, the file must be committed there. @@ -299,7 +299,7 @@ CI focuses on lint, typecheck, build, Flatpak metadata validation, and coverage - Verify the tag follows semantic versioning (`v1.2.3`) - Ensure `GH_TOKEN` secret is set in repository settings -- Check that `dist:*:publish` scripts exist in `package.json` +- Check that `dist:*` / `dist:*:publish` scripts exist in `package.json` (tag release CI uses `dist:*` + `ci-upload-release-assets.mjs`) ### Docs deployment fails diff --git a/docs/release-process.md b/docs/release-process.md index f22aabab1..c50dc5fa5 100644 --- a/docs/release-process.md +++ b/docs/release-process.md @@ -8,14 +8,14 @@ This document describes how maintainers create releases for Mesh-Client. Releases are driven by **annotated version tags** (`v*`) on `main`. Pushing a tag triggers: -| Workflow | Purpose | -| --------------------------------------------------- | --------------------------------------------------------------------------------------------- | -| [`release.yaml`](../.github/workflows/release.yaml) | Build and publish macOS, Linux, and Windows installers via `electron-builder` | -| [`flatpak.yaml`](../.github/workflows/flatpak.yaml) | Build Reticulum sidecar + Flatpak bundles (x86_64 and aarch64) and attach them to the release | +| Workflow | Purpose | +| --------------------------------------------------- | ---------------------------------------------------------------------------------------------------- | +| [`release.yaml`](../.github/workflows/release.yaml) | Build macOS/Linux/Windows via `electron-builder` (`--publish never`) and attach to the prepare draft | +| [`flatpak.yaml`](../.github/workflows/flatpak.yaml) | Build Reticulum sidecar + Flatpak bundles (x86_64 and aarch64) and attach them to the same draft | Both workflows upload to a **draft** GitHub Release. A maintainer reviews artifacts and publishes manually when ready. -`electron-builder.yml` sets `releaseType: draft`, so the Electron jobs also create/update a draft release rather than publishing live immediately. +`prepare-github-release` is the **only** job that creates the draft (`MESH_CLIENT_ALLOW_DRAFT_CREATE=1`). Matrix builds and Flatpak attach with `ci-upload-release-assets.mjs` by `release_id` so parallel jobs cannot fork duplicate drafts. `electron-builder.yml` still sets `releaseType: draft` for local `dist:*:publish` use. Documentation deploys separately: [`docs.yml`](../.github/workflows/docs.yml) runs on every push to `main` (including the version-bump commit from `pnpm run release`). @@ -150,11 +150,11 @@ git push origin vX.Y.Z Matrix build jobs: -- **`macos-latest`** → `pnpm run dist:mac:publish` -- **`ubuntu-latest`** → `pnpm run dist:linux:publish` (x64 + arm64 AppImage, `.deb`, `.rpm`) -- **`windows-latest`** → `pnpm run dist:win:publish` (x64 + arm64 NSIS installers) +- **`macos-latest`** → `pnpm run dist:mac` then `ci-upload-release-assets.mjs` +- **`ubuntu-latest`** → `pnpm run dist:linux` (x64 + arm64 AppImage, `.deb`, `.rpm`) then upload +- **`windows-latest`** → `pnpm run dist:win` (x64 + arm64 NSIS installers) then upload -Each job runs `pnpm install --frozen-lockfile`, `pnpm run rebuild`, then publishes via `electron-builder` using the built-in **`GITHUB_TOKEN`** (exported as `GH_TOKEN` for electron-publish). +Each job runs `pnpm install --frozen-lockfile`, `pnpm run rebuild`, builds with `--publish never`, then attaches artifacts to the prepare draft with **`GITHUB_TOKEN`** as `GH_TOKEN`. After builds finish, **`packaging-smoke`** runs on: @@ -245,17 +245,17 @@ Follow [Semantic Versioning](https://semver.org/): - Platform failures are often native-module or packaging related - Fix on `main`, then cut a new patch release (`pnpm run release patch`) -### Electron-builder fails to publish +### Upload to draft release fails -- Confirm the workflow job has `contents: write` -- Publishing uses `GITHUB_TOKEN` as `GH_TOKEN`; forked or restricted workflows may lack upload permission -- **404 uploading to `/releases/{id}/assets`:** parallel `dist:*:publish` jobs raced and created duplicate draft releases. Re-run the failed release workflow after merging the `prepare-github-release` gate (or delete orphan drafts and re-run). Do not PATCH release `tag_name` via API while CI is uploading — that orphans in-flight upload targets. +- Confirm the workflow job has `contents: write` and `RELEASE_ID` is set from `prepare-github-release` +- Uploads use `GITHUB_TOKEN` as `GH_TOKEN` via `ci-upload-release-assets.mjs`; forked or restricted workflows may lack upload permission +- Tag CI builds with `dist:*` (`--publish never`) and attaches by id — do **not** reintroduce `dist:*:publish` in `release.yaml` (electron-builder `POST /releases` forks drafts) ### Duplicate draft releases for one tag -- Caused when GitHub’s `GET /releases/tags/{tag}` returns **404** while multiple draft releases share the same `tag_name` — parallel `dist:*:publish` jobs each create another draft. `release.yaml` runs `scripts/ci-ensure-github-draft-release.mjs` before builds and again in `finalize-github-release` (list + merge split assets + delete duplicates + set `target_commitish`). -- **Assets spread across duplicates:** CI now merges automatically; for a broken tag outside CI, run `node scripts/consolidate-github-release-duplicates.mjs --tag vX.Y.Z` (requires `GH_TOKEN`), then re-run the release workflow for any missing platform artifacts. -- To recover on a broken tag: consolidate duplicates, keep the draft with merged assets, re-run **Build/Release Electron App** on the tag. +- Historically caused when parallel `dist:*:publish` / softprops jobs each `POST`ed a draft after a List Releases miss. Current CI: only `prepare-github-release` may create (`MESH_CLIENT_ALLOW_DRAFT_CREATE=1`); builds/Flatpak upload by id; Flatpak waits with `ci-wait-github-draft-release.mjs`. +- **Finalize PATCH 403 (`Resource not accessible by integration`):** Actions `GITHUB_TOKEN` cannot PATCH `target_commitish` when the tagged commit differs in `.github/workflows/` from the default branch. Consolidation skips that field and treats metadata PATCH failures as non-fatal after assets are merged. +- **Assets still split (external fork):** `finalize-github-release` merges via `ci-ensure-github-draft-release.mjs`; outside CI run `node scripts/consolidate-github-release-duplicates.mjs --tag vX.Y.Z` (requires `GH_TOKEN`). - **Do not force-move the `v*` tag while a release workflow is in progress.** Retagging starts another run and (with workflow concurrency) cancels the in-flight build; smoke jobs also assume a stable workflow `github.sha`. - **Smoke tests fail with “ref does not point to the expected commit”:** the tag was moved after the workflow started. Re-run failed jobs only after the tag matches the run’s `headSha`, or merge the checkout `ref: ${{ github.sha }}` fix and trigger a fresh tag run. diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 1a82beea8..e2faa861c 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -154,7 +154,7 @@ Optional persistent mitigation: ### `pnpm run dist:mac` fails with `GH_TOKEN` / "Cannot cleanup" -electron-builder publishes to GitHub when it thinks it's in CI. Local builds use `--publish never` so artifacts land in `release/` without a token. Tag releases use `pnpm run dist:mac:publish` (and `:linux:publish` / `:win:publish`) with `GH_TOKEN` set; see `.github/workflows/release.yaml`. +electron-builder publishes to GitHub when it thinks it's in CI. Local builds use `--publish never` so artifacts land in `release/` without a token. Tag release CI also builds with `dist:*` (`--publish never`) and attaches via `ci-upload-release-assets.mjs` to the prepare draft; see `.github/workflows/release.yaml`. ### `[DEP0190]` when running electron-builder diff --git a/scripts/ci-ensure-github-draft-release.mjs b/scripts/ci-ensure-github-draft-release.mjs index 8c56e8d52..cab4f2d2f 100644 --- a/scripts/ci-ensure-github-draft-release.mjs +++ b/scripts/ci-ensure-github-draft-release.mjs @@ -1,4 +1,5 @@ #!/usr/bin/env node +import { appendFileSync } from 'node:fs'; import { pathToFileURL } from 'node:url'; import { authToken, @@ -7,14 +8,29 @@ import { resolveTargetCommitish, } from './github-release-api.mjs'; +/** + * @param {string | undefined} githubOutput + * @param {number | string} releaseId + */ +export function writeReleaseIdOutput(githubOutput, releaseId) { + if (typeof githubOutput !== 'string' || !githubOutput) { + return; + } + appendFileSync(githubOutput, `release_id=${releaseId}\n`, 'utf8'); +} + async function main() { const tag = resolveTag(process.argv.slice(2), process.env); const token = authToken(process.env); - await ensureGithubDraftRelease({ + const allowCreate = process.env.MESH_CLIENT_ALLOW_DRAFT_CREATE === '1'; + const release = await ensureGithubDraftRelease({ tag, token, targetCommitish: resolveTargetCommitish(process.env), + allowCreate, }); + writeReleaseIdOutput(process.env.GITHUB_OUTPUT, release.id); + console.debug(`[ci-ensure-github-draft-release] release_id=${release.id}`); } const entry = process.argv[1] ? pathToFileURL(process.argv[1]).href : ''; diff --git a/scripts/ci-ensure-github-draft-release.test.mjs b/scripts/ci-ensure-github-draft-release.test.mjs index 562872a89..29118acae 100644 --- a/scripts/ci-ensure-github-draft-release.test.mjs +++ b/scripts/ci-ensure-github-draft-release.test.mjs @@ -7,6 +7,7 @@ import { normalizeDraftReleasesForTag, pickCanonicalRelease, resolveTag, + waitForGithubDraftRelease, } from './github-release-api.mjs'; const TAG = 'v5.21.0'; @@ -122,7 +123,7 @@ describe('ensureGithubDraftRelease', () => { expect(fetchMock.mock.calls.some(([, init]) => init?.method === 'DELETE')).toBe(true); }); - it('creates a draft when no release exists', async () => { + it('creates a draft when no release exists and allowCreate is true', async () => { const fetchMock = vi.fn(async (url, init) => { const method = init?.method ?? 'GET'; const href = String(url); @@ -141,13 +142,14 @@ describe('ensureGithubDraftRelease', () => { const release = await ensureGithubDraftRelease({ tag: TAG, token: 'test-token', + allowCreate: true, log: () => {}, }); expect(release.id).toBe(99); }); - it('creates a draft when only a published release matches the tag', async () => { + it('creates a draft when only a published release matches and allowCreate is true', async () => { const fetchMock = vi.fn(async (url, init) => { const method = init?.method ?? 'GET'; const href = String(url); @@ -169,12 +171,30 @@ describe('ensureGithubDraftRelease', () => { const release = await ensureGithubDraftRelease({ tag: TAG, token: 'test-token', + allowCreate: true, log: () => {}, }); expect(release.id).toBe(99); expect(release.draft).toBe(true); }); + + it('does not create when allowCreate is false and no draft exists', async () => { + const exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => undefined); + const fetchMock = vi.fn(async () => new Response(JSON.stringify([]), { status: 200 })); + vi.stubGlobal('fetch', fetchMock); + + await ensureGithubDraftRelease({ + tag: TAG, + token: 'test-token', + allowCreate: false, + log: () => {}, + }); + + expect(exitSpy).toHaveBeenCalledWith(1); + expect(fetchMock.mock.calls.some(([, init]) => (init?.method ?? 'GET') === 'POST')).toBe(false); + exitSpy.mockRestore(); + }); }); describe('normalizeDraftReleasesForTag', () => { @@ -277,6 +297,33 @@ describe('normalizeDraftReleasesForTag', () => { }); }); +describe('waitForGithubDraftRelease', () => { + it('returns the draft after a list miss then hit', async () => { + const fetchMock = vi + .fn() + .mockResolvedValueOnce(new Response(JSON.stringify([]), { status: 200 })) + .mockResolvedValueOnce( + new Response( + JSON.stringify([{ id: 7, tag_name: TAG, name: '5.21.0', draft: true, assets: [] }]), + { status: 200 }, + ), + ); + vi.stubGlobal('fetch', fetchMock); + + const release = await waitForGithubDraftRelease({ + tag: TAG, + token: 'token', + attempts: 3, + delayMs: 1, + sleep: async () => {}, + log: () => {}, + }); + + expect(release.id).toBe(7); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); +}); + describe('consolidateReleases', () => { it('is a no-op when only one release exists', async () => { const fetchMock = vi.fn( @@ -292,4 +339,73 @@ describe('consolidateReleases', () => { expect(release.id).toBe(1); expect(fetchMock.mock.calls.some(([, init]) => init?.method === 'DELETE')).toBe(false); }); + + it('treats metadata PATCH 403 as non-fatal after assets are merged', async () => { + const logs = []; + const fetchMock = vi.fn(async (url, init) => { + const method = init?.method ?? 'GET'; + const href = String(url); + if (method === 'GET' && href.includes('/releases?')) { + return new Response( + JSON.stringify([ + { + id: 1, + tag_name: TAG, + name: '5.21.0', + draft: true, + body: '', + assets: [ + { id: 101, name: 'a' }, + { id: 103, name: 'c' }, + ], + }, + { + id: 2, + tag_name: TAG, + name: '5.21.0', + draft: true, + body: 'longer body from duplicate', + assets: [{ id: 102, name: 'b' }], + }, + ]), + { status: 200 }, + ); + } + if (method === 'GET' && href.endsWith('/releases/assets/102')) { + return new Response(new Uint8Array([1, 2, 3]), { status: 200 }); + } + if (method === 'POST' && href.includes('/releases/1/assets')) { + return new Response(JSON.stringify({ id: 999, name: 'b' }), { status: 201 }); + } + if ( + method === 'DELETE' && + (href.endsWith('/releases/assets/102') || href.endsWith('/releases/2')) + ) { + return new Response('', { status: 200 }); + } + if (method === 'PATCH' && href.endsWith('/releases/1')) { + return new Response(JSON.stringify({ message: 'Resource not accessible by integration' }), { + status: 403, + }); + } + throw new Error(`Unexpected fetch ${method} ${href}`); + }); + vi.stubGlobal('fetch', fetchMock); + + const release = await consolidateReleases({ + tag: TAG, + token: 'token', + targetCommitish: 'a'.repeat(40), + log: (message) => logs.push(message), + }); + + expect(release.id).toBe(1); + expect(logs.some((line) => line.includes('PATCH release 1 failed (403)'))).toBe(true); + const patchBody = JSON.parse( + fetchMock.mock.calls.find( + ([url, init]) => init?.method === 'PATCH' && String(url).endsWith('/releases/1'), + )?.[1]?.body ?? '{}', + ); + expect(patchBody.target_commitish).toBeUndefined(); + }); }); diff --git a/scripts/ci-patch-draft-release-schema-note.mjs b/scripts/ci-patch-draft-release-schema-note.mjs index 442984adc..570c40314 100644 --- a/scripts/ci-patch-draft-release-schema-note.mjs +++ b/scripts/ci-patch-draft-release-schema-note.mjs @@ -15,6 +15,7 @@ import { import { authToken, ensureGithubDraftRelease, + getRelease, listReleasesForTag, patchRelease, resolveTag, @@ -73,34 +74,60 @@ export function schemaMarkdownFromCompareOutputs(env = process.env) { }); } +/** + * @param {string | undefined} raw + * @returns {number | undefined} + */ +export function parseOptionalReleaseId(raw) { + if (typeof raw !== 'string' || raw === '') { + return undefined; + } + if (!/^\d+$/.test(raw)) { + throw new Error(`RELEASE_ID must be numeric (got ${JSON.stringify(raw)})`); + } + return Number(raw); +} + /** * @param {{ * tag: string, * token: string, * markdown: string, + * releaseId?: number, * targetCommitish?: string, * ensureDraft?: typeof ensureGithubDraftRelease, + * getReleaseById?: typeof getRelease, * listReleases?: typeof listReleasesForTag, * patch?: typeof patchRelease, * }} opts */ export async function patchDraftReleaseSchemaNote(opts) { const ensureDraft = opts.ensureDraft ?? ensureGithubDraftRelease; + const getReleaseById = opts.getReleaseById ?? getRelease; const listReleases = opts.listReleases ?? listReleasesForTag; const patch = opts.patch ?? patchRelease; - // Prefer the draft returned by ensure (POST/PATCH response). Re-listing immediately - // after create can miss the draft — GitHub List Releases lags briefly, which caused - // prepare-github-release to fail with "No release found" right after creating one. - const ensured = await ensureDraft({ - tag: opts.tag, - token: opts.token, - targetCommitish: opts.targetCommitish, - }); - const draft = - ensured?.draft === true - ? ensured - : requireDraftReleaseForSchemaPatch(await listReleases(opts.tag, opts.token), opts.tag); + /** @type {{ id: number, draft?: boolean, body?: string | null }} */ + let draft; + if (opts.releaseId != null) { + // Prefer prepare's release_id — avoids List Releases lag right after create. + draft = await getReleaseById(opts.releaseId, opts.token); + if (draft.draft !== true) { + throw new Error(`No release found for ${opts.tag}`); + } + } else { + const ensured = await ensureDraft({ + tag: opts.tag, + token: opts.token, + targetCommitish: opts.targetCommitish, + allowCreate: false, + }); + draft = + ensured?.draft === true + ? ensured + : requireDraftReleaseForSchemaPatch(await listReleases(opts.tag, opts.token), opts.tag); + } + const body = mergeSchemaNoteIntoReleaseBody(draft.body ?? '', opts.markdown); await patch(draft.id, opts.token, { body }); console.debug(`[ci-patch-draft-release-schema-note] Updated draft body for ${opts.tag}`); @@ -121,6 +148,7 @@ async function main() { tag, token, markdown, + releaseId: parseOptionalReleaseId(process.env.RELEASE_ID), targetCommitish: resolveTargetCommitish(process.env), }); } diff --git a/scripts/ci-patch-draft-release-schema-note.test.mjs b/scripts/ci-patch-draft-release-schema-note.test.mjs index c7c00d660..c9ac26363 100644 --- a/scripts/ci-patch-draft-release-schema-note.test.mjs +++ b/scripts/ci-patch-draft-release-schema-note.test.mjs @@ -90,22 +90,26 @@ describe('patchDraftReleaseSchemaNote', () => { expect(patch).not.toHaveBeenCalled(); }); - it('PATCHes using the ensure return value without re-listing', async () => { + it('PATCHes using RELEASE_ID without ensure or list', async () => { const patch = vi.fn().mockResolvedValue({ id: 2 }); + const ensureDraft = vi.fn(); const listReleases = vi.fn(); await patchDraftReleaseSchemaNote({ tag: 'v1.0.0', token: 'token', markdown: '# Schema bumped', - ensureDraft: vi.fn().mockResolvedValue({ + releaseId: 2, + ensureDraft, + listReleases, + getReleaseById: vi.fn().mockResolvedValue({ id: 2, draft: true, body: 'Draft release for v1.0.0.\n', }), - listReleases, patch, }); + expect(ensureDraft).not.toHaveBeenCalled(); expect(listReleases).not.toHaveBeenCalled(); expect(patch).toHaveBeenCalledTimes(1); expect(patch.mock.calls[0][0]).toBe(2); diff --git a/scripts/ci-upload-release-assets.mjs b/scripts/ci-upload-release-assets.mjs new file mode 100644 index 000000000..fc31ad9ed --- /dev/null +++ b/scripts/ci-upload-release-assets.mjs @@ -0,0 +1,122 @@ +#!/usr/bin/env node +/** + * Upload local files to an existing GitHub release by id. + * Never creates a release (prevents duplicate draft forks from electron-builder / softprops). + */ +import { readFileSync, globSync, statSync } from 'node:fs'; +import path from 'node:path'; +import { pathToFileURL } from 'node:url'; +import { authToken, fail, getRelease, uploadOrReplaceReleaseAsset } from './github-release-api.mjs'; + +/** + * @param {string | undefined} raw + */ +export function parseReleaseId(raw) { + if (typeof raw !== 'string' || !/^\d+$/.test(raw)) { + fail(`RELEASE_ID must be a numeric GitHub release id (got ${JSON.stringify(raw)})`); + } + return Number(raw); +} + +/** + * Expand CLI args (paths or globs) to unique regular files. + * @param {string[]} patterns + * @param {string} [cwd] + */ +export function resolveUploadFiles(patterns, cwd = process.cwd()) { + if (!Array.isArray(patterns) || patterns.length === 0) { + fail('Usage: ci-upload-release-assets.mjs ...'); + } + /** @type {Set} */ + const files = new Set(); + for (const pattern of patterns) { + const matches = globSync(pattern, { + cwd, + absolute: true, + nodir: true, + dot: false, + }); + if (matches.length === 0) { + const abs = path.resolve(cwd, pattern); + try { + if (statSync(abs).isFile()) { + files.add(abs); + continue; + } + } catch { + // catch-no-log-ok missing path checked below via empty matches + } + fail(`No files matched upload pattern: ${pattern}`); + } + for (const match of matches) { + files.add(match); + } + } + return [...files].sort((a, b) => a.localeCompare(b)); +} + +/** + * @param {{ + * releaseId: number, + * token: string, + * files: string[], + * get?: typeof getRelease, + * upload?: typeof uploadOrReplaceReleaseAsset, + * readFile?: (path: string) => Uint8Array, + * log?: (...args: unknown[]) => void, + * }} opts + */ +export async function uploadReleaseAssets(opts) { + const get = opts.get ?? getRelease; + const upload = opts.upload ?? uploadOrReplaceReleaseAsset; + const readFile = opts.readFile ?? ((filePath) => new Uint8Array(readFileSync(filePath))); + const log = opts.log ?? console.debug; + + const release = await get(opts.releaseId, opts.token); + if (release.draft !== true) { + fail(`Release ${opts.releaseId} is not a draft; refusing to upload`); + return 0; + } + + /** @type {Array<{ id: number, name: string }>} */ + let existingAssets = [...(release.assets ?? [])]; + let uploaded = 0; + + for (const filePath of opts.files) { + const fileName = path.basename(filePath); + const bytes = readFile(filePath); + log( + `[ci-upload-release-assets] Uploading ${fileName} (${bytes.byteLength} bytes) → release ${opts.releaseId}`, + ); + await upload({ + releaseId: opts.releaseId, + token: opts.token, + fileName, + bytes, + existingAssets, + log, + }); + existingAssets = existingAssets.filter((asset) => asset.name !== fileName); + existingAssets.push({ id: -1, name: fileName }); + uploaded += 1; + } + + log(`[ci-upload-release-assets] Uploaded ${uploaded} asset(s) to release ${opts.releaseId}`); + return uploaded; +} + +async function main() { + const releaseId = parseReleaseId(process.env.RELEASE_ID); + const token = authToken(process.env); + const files = resolveUploadFiles(process.argv.slice(2)); + await uploadReleaseAssets({ releaseId, token, files }); +} + +const entry = process.argv[1] ? pathToFileURL(process.argv[1]).href : ''; +if (entry && import.meta.url === entry) { + main().catch((error) => { + const detail = error instanceof Error ? error.message : String(error); + console.error(`[ci-upload-release-assets] ${detail}`); + process.exit(1); + }); +} diff --git a/scripts/ci-upload-release-assets.test.mjs b/scripts/ci-upload-release-assets.test.mjs new file mode 100644 index 000000000..95d43878e --- /dev/null +++ b/scripts/ci-upload-release-assets.test.mjs @@ -0,0 +1,77 @@ +import { mkdtempSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { + parseReleaseId, + resolveUploadFiles, + uploadReleaseAssets, +} from './ci-upload-release-assets.mjs'; + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe('parseReleaseId', () => { + it('accepts numeric ids', () => { + expect(parseReleaseId('368221738')).toBe(368221738); + }); + + it('rejects non-numeric ids', () => { + const exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => undefined); + parseReleaseId('untagged'); + expect(exitSpy).toHaveBeenCalledWith(1); + exitSpy.mockRestore(); + }); +}); + +describe('resolveUploadFiles', () => { + it('expands globs to absolute files', () => { + const dir = mkdtempSync(path.join(tmpdir(), 'mesh-upload-')); + writeFileSync(path.join(dir, 'a.deb'), 'a'); + writeFileSync(path.join(dir, 'b.rpm'), 'b'); + const files = resolveUploadFiles(['*.deb', '*.rpm'], dir); + expect(files.map((file) => path.basename(file)).sort()).toEqual(['a.deb', 'b.rpm']); + }); +}); + +describe('uploadReleaseAssets', () => { + it('refuses non-draft releases', async () => { + const exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => undefined); + const upload = vi.fn(); + await uploadReleaseAssets({ + releaseId: 1, + token: 'token', + files: ['/tmp/x.deb'], + get: async () => ({ id: 1, draft: false, assets: [] }), + upload, + log: () => {}, + }); + expect(exitSpy).toHaveBeenCalledWith(1); + expect(upload).not.toHaveBeenCalled(); + exitSpy.mockRestore(); + }); + + it('uploads each file to a draft release', async () => { + const uploads = []; + const count = await uploadReleaseAssets({ + releaseId: 9, + token: 'token', + files: ['/tmp/a.deb', '/tmp/b.yml'], + get: async () => ({ + id: 9, + draft: true, + assets: [{ id: 3, name: 'a.deb' }], + }), + readFile: (filePath) => new Uint8Array(Buffer.from(path.basename(filePath))), + upload: async (opts) => { + uploads.push(opts.fileName); + return { id: 1, name: opts.fileName }; + }, + log: () => {}, + }); + + expect(count).toBe(2); + expect(uploads).toEqual(['a.deb', 'b.yml']); + }); +}); diff --git a/scripts/ci-wait-github-draft-release.mjs b/scripts/ci-wait-github-draft-release.mjs new file mode 100644 index 000000000..96eaf2f60 --- /dev/null +++ b/scripts/ci-wait-github-draft-release.mjs @@ -0,0 +1,27 @@ +#!/usr/bin/env node +/** + * Wait for prepare-github-release to create the draft, then export release_id. + * Used by Flatpak publish so it never POSTs a competing draft. + */ +import { appendFileSync } from 'node:fs'; +import { pathToFileURL } from 'node:url'; +import { authToken, resolveTag, waitForGithubDraftRelease } from './github-release-api.mjs'; + +async function main() { + const tag = resolveTag(process.argv.slice(2), process.env); + const token = authToken(process.env); + const release = await waitForGithubDraftRelease({ tag, token }); + if (typeof process.env.GITHUB_OUTPUT === 'string' && process.env.GITHUB_OUTPUT) { + appendFileSync(process.env.GITHUB_OUTPUT, `release_id=${release.id}\n`, 'utf8'); + } + console.debug(`[ci-wait-github-draft-release] release_id=${release.id}`); +} + +const entry = process.argv[1] ? pathToFileURL(process.argv[1]).href : ''; +if (entry && import.meta.url === entry) { + main().catch((error) => { + const detail = error instanceof Error ? error.message : String(error); + console.error(`[ci-wait-github-draft-release] ${detail}`); + process.exit(1); + }); +} diff --git a/scripts/github-release-api.mjs b/scripts/github-release-api.mjs index f023f2645..234f3001c 100644 --- a/scripts/github-release-api.mjs +++ b/scripts/github-release-api.mjs @@ -215,6 +215,43 @@ export async function patchRelease(releaseId, token, patch) { return json; } +/** + * PATCH release metadata, returning null on failure instead of exiting. + * Used after asset consolidation so a metadata 403 cannot undo a successful merge. + * + * Do not send `target_commitish` here: Actions `GITHUB_TOKEN` cannot retarget a release + * when the commit differs in `.github/workflows/` from the default branch (HTTP 403 + * "Resource not accessible by integration"). The git tag already pins the SHA. + */ +export async function patchReleaseMetadataBestEffort(releaseId, token, patch, log = console.debug) { + if (!patch || Object.keys(patch).length === 0) { + return null; + } + const { response, json } = await githubRequest(`/releases/${releaseId}`, { + token, + method: 'PATCH', + body: patch, + }); + if (!response.ok) { + log( + `[github-release] PATCH release ${releaseId} failed (${response.status}): ` + + `${json?.message ?? response.statusText} — leaving metadata unchanged after asset merge`, + ); + return null; + } + return json; +} + +export async function getRelease(releaseId, token) { + const { response, json } = await githubRequest(`/releases/${releaseId}`, { token }); + if (!response.ok) { + fail( + `GET release ${releaseId} failed (${response.status}): ${json?.message ?? response.statusText}`, + ); + } + return json; +} + export async function uploadReleaseAsset(releaseId, fileName, bytes, token) { const uploadUrl = `https://uploads.github.com/repos/${OWNER}/${REPO}/releases/${releaseId}/assets?name=${encodeURIComponent(fileName)}`; const response = await fetch(uploadUrl, { @@ -247,6 +284,34 @@ export async function uploadReleaseAsset(releaseId, fileName, bytes, token) { return json; } +/** + * Upload (or replace) a single asset on an existing release. Never creates a release. + * @param {{ + * releaseId: number, + * token: string, + * fileName: string, + * bytes: Uint8Array, + * existingAssets?: Array<{ id: number, name: string }>, + * log?: (...args: unknown[]) => void, + * }} opts + */ +export async function uploadOrReplaceReleaseAsset({ + releaseId, + token, + fileName, + bytes, + existingAssets, + log = console.debug, +}) { + const assets = existingAssets ?? (await getRelease(releaseId, token)).assets ?? []; + const prior = assets.find((asset) => asset.name === fileName); + if (prior) { + log(`[github-release] Replacing existing asset ${fileName} on release ${releaseId}`); + await deleteReleaseAsset(prior.id, token); + } + return uploadReleaseAsset(releaseId, fileName, bytes, token); +} + /** * Delete empty duplicate draft releases for a tag. Returns the canonical release, if any. * @deprecated Prefer normalizeDraftReleasesForTag, which also merges duplicates that hold assets. @@ -270,7 +335,17 @@ export async function downloadReleaseAsset(assetId, token) { return new Uint8Array(await response.arrayBuffer()); } -export async function consolidateReleases({ tag, token, targetCommitish, log = console.debug }) { +/** + * @param {{ + * tag: string, + * token: string, + * targetCommitish?: string, + * log?: (...args: unknown[]) => void, + * }} opts + * `targetCommitish` is accepted for call-site compatibility but never PATCHed + * (GITHUB_TOKEN 403 — see patchReleaseMetadataBestEffort). + */ +export async function consolidateReleases({ tag, token, log = console.debug }) { const releases = await listReleasesForTag(tag, token); if (releases.length === 0) { fail(`No releases found for ${tag}`); @@ -325,36 +400,39 @@ export async function consolidateReleases({ tag, token, targetCommitish, log = c name: versionFromTag(tag), draft: true, }; - if (targetCommitish) { - patch.target_commitish = targetCommitish; - } if (bestBody && bestBody !== keeper.body) { patch.body = bestBody; } - const updated = await patchRelease(keeper.id, token, patch); + const updated = await patchReleaseMetadataBestEffort(keeper.id, token, patch, log); + const result = updated ?? keeper; log( - `[github-release] Consolidated ${tag} — release ${updated.id} has ${updated.assets?.length ?? keeperAssetNames.size} assets (moved ${moved})`, + `[github-release] Consolidated ${tag} — release ${result.id} has ${result.assets?.length ?? keeperAssetNames.size} assets (moved ${moved})`, ); - return updated; + return result; } /** * Ensure at most one draft release exists for a tag. Merges split assets when parallel * publish jobs forked duplicate drafts (including untagged-e* names matched by release name). + * + * @param {string} tag + * @param {string} token + * @param {{ + * targetCommitish?: string, + * log?: (...args: unknown[]) => void, + * }} [opts] + * `targetCommitish` is accepted for call-site compatibility but never PATCHed + * (GITHUB_TOKEN 403 — see patchReleaseMetadataBestEffort). */ -export async function normalizeDraftReleasesForTag( - tag, - token, - { targetCommitish, log = console.debug } = {}, -) { +export async function normalizeDraftReleasesForTag(tag, token, { log = console.debug } = {}) { const releases = await listReleasesForTag(tag, token); if (releases.length === 0) { return null; } if (releases.length > 1) { - return consolidateReleases({ tag, token, targetCommitish, log }); + return consolidateReleases({ tag, token, log }); } const release = releases[0]; @@ -364,26 +442,37 @@ export async function normalizeDraftReleasesForTag( patch.name = versionFromTag(tag); patch.draft = true; } - if (targetCommitish && release.target_commitish !== targetCommitish) { - patch.target_commitish = targetCommitish; - patch.draft = true; - } if (Object.keys(patch).length === 0) { return release; } - const updated = await patchRelease(release.id, token, patch); + const updated = await patchReleaseMetadataBestEffort(release.id, token, patch, log); + if (!updated) { + return release; + } log(`[github-release] Repaired release ${updated.id} metadata for ${tag}`); return updated; } +/** + * @param {{ + * tag: string, + * token: string, + * targetCommitish?: string, + * allowCreate?: boolean, + * log?: (...args: unknown[]) => void, + * }} opts + * Create only when `allowCreate` is true (prepare job sets MESH_CLIENT_ALLOW_DRAFT_CREATE=1). + * Upload jobs must reuse the prepare draft and never POST /releases. + */ export async function ensureGithubDraftRelease({ tag, token, targetCommitish, + allowCreate = false, log = console.debug, }) { - let keeper = await normalizeDraftReleasesForTag(tag, token, { targetCommitish, log }); + let keeper = await normalizeDraftReleasesForTag(tag, token, { log }); // Only reuse an existing *draft*. A published release for the same tag must not be // treated as the CI upload target (schema-note patches and artifact uploads are draft-only). if (keeper?.draft === true) { @@ -391,7 +480,54 @@ export async function ensureGithubDraftRelease({ return keeper; } + if (!allowCreate) { + fail( + `No draft release for ${tag}. prepare-github-release must create it first ` + + `(set MESH_CLIENT_ALLOW_DRAFT_CREATE=1 only in that job).`, + ); + return /** @type {never} */ (undefined); + } + keeper = await createDraftRelease(tag, token, targetCommitish); log(`[ci-ensure-github-draft-release] Created draft release ${keeper.id} for ${tag}`); return keeper; } + +/** + * Poll until a draft exists for the tag (Flatpak may start before Electron prepare finishes). + * @param {{ + * tag: string, + * token: string, + * attempts?: number, + * delayMs?: number, + * sleep?: (ms: number) => Promise, + * log?: (...args: unknown[]) => void, + * }} opts + */ +export async function waitForGithubDraftRelease({ + tag, + token, + attempts = 30, + delayMs = 10_000, + sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms)), + log = console.debug, +}) { + assertSafeReleaseTag(tag); + for (let attempt = 1; attempt <= attempts; attempt += 1) { + const releases = await listReleasesForTag(tag, token); + const draft = releases.find((release) => release.draft === true); + if (draft) { + log( + `[github-release] Found draft release ${draft.id} for ${tag} (attempt ${attempt}/${attempts})`, + ); + return draft; + } + if (attempt < attempts) { + log( + `[github-release] No draft for ${tag} yet (attempt ${attempt}/${attempts}); waiting ${delayMs}ms`, + ); + await sleep(delayMs); + } + } + fail(`Timed out waiting for draft release ${tag} after ${attempts} attempts`); +} diff --git a/scripts/resolve-release-matrix.mjs b/scripts/resolve-release-matrix.mjs index c11c9b99c..b3dd4347b 100644 --- a/scripts/resolve-release-matrix.mjs +++ b/scripts/resolve-release-matrix.mjs @@ -7,21 +7,27 @@ const ROWS = [ { os: 'macos-latest', platform_key: 'mac', - build_script: 'pnpm run dist:mac:publish', + // Build only — GitHub upload is ci-upload-release-assets.mjs (never POST /releases). + build_script: 'pnpm run dist:mac', + upload_globs: + 'release/*.dmg release/*.zip release/*.blockmap release/latest-mac.yml release/mac*/*.dmg release/mac*/*.zip release/mac*/*.blockmap', sidecar_platform: 'darwin', rust_targets: 'aarch64-apple-darwin', }, { os: 'ubuntu-latest', platform_key: 'linux', - build_script: 'pnpm run dist:linux:publish', + build_script: 'pnpm run dist:linux', + upload_globs: + 'release/*.AppImage release/*.rpm release/*.deb release/*.blockmap release/latest-linux.yml release/latest-linux-arm64.yml', sidecar_platform: 'linux', rust_targets: 'x86_64-unknown-linux-gnu,aarch64-unknown-linux-gnu', }, { os: 'windows-latest', platform_key: 'win', - build_script: 'pnpm run dist:win:publish', + build_script: 'pnpm run dist:win', + upload_globs: 'release/*.exe release/*.blockmap release/latest.yml', sidecar_platform: 'win32', rust_targets: 'x86_64-pc-windows-msvc,aarch64-pc-windows-msvc', }, From 6de6f651df54a49fb9d8fcc2805878b20501f2bc Mon Sep 17 00:00:00 2001 From: Joey Stanford Date: Mon, 10 Aug 2026 17:10:08 -0600 Subject: [PATCH 2/2] fix(ci): address release draft PR review and packaging contract Gate matrix uploads on a successful prepare release_id, harden upload globs/basenames, soft-fail finalize metadata PATCH only on HTTP 403, and align docs/tests with the upload-by-id Flatpak/matrix flow. --- .github/workflows/release.yaml | 9 ++- docs/release-process.md | 8 +-- .../ci-ensure-github-draft-release.test.mjs | 55 +++++++++++++++++++ .../ci-patch-draft-release-schema-note.mjs | 2 +- ...i-patch-draft-release-schema-note.test.mjs | 21 +++++++ scripts/ci-upload-release-assets.mjs | 33 ++++++++++- scripts/ci-upload-release-assets.test.mjs | 37 ++++++++++++- scripts/ci-wait-github-draft-release.mjs | 6 +- scripts/github-release-api.mjs | 14 ++++- src/main/windows-packaging.contract.test.ts | 2 +- 10 files changed, 168 insertions(+), 19 deletions(-) diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 683b4a101..323393aa3 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -239,12 +239,19 @@ jobs: run: ${{ matrix.build_script }} - name: Upload assets to draft GitHub release + if: >- + ${{ needs.prepare-github-release.result == 'success' + && needs.prepare-github-release.outputs.release_id != '' }} shell: bash env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} RELEASE_ID: ${{ needs.prepare-github-release.outputs.release_id }} + UPLOAD_GLOBS: ${{ matrix.upload_globs }} # Attach only — never create a release (prepare owns POST /releases). - run: node scripts/ci-upload-release-assets.mjs ${{ matrix.upload_globs }} + # UPLOAD_GLOBS is an intentional space-separated glob list from the matrix. + run: | + # shellcheck disable=SC2086 + node scripts/ci-upload-release-assets.mjs $UPLOAD_GLOBS # Do not upload raw Mesh-client.app — upload-artifact@v7 dereferences framework # symlinks (~3× Electron Framework size). DMG/ZIP preserve correct layout. diff --git a/docs/release-process.md b/docs/release-process.md index c50dc5fa5..c9c3ca392 100644 --- a/docs/release-process.md +++ b/docs/release-process.md @@ -170,7 +170,7 @@ Build jobs also run `verify-reticulum-sidecar-staged.mjs` after staging sidecars 1. **`schema-release-compare`** — compares this SHA’s schema to the last published release; uploads `READ-ME-FIRST-flatpak.md` (included again beside Flatpak Actions artifacts) 2. **`reticulum-sidecar`** — builds `mesh-client-reticulum` per arch (x86_64 on `ubuntu-latest`, aarch64 on `ubuntu-24.04-arm`) with full RNS stack features 3. **`flatpak`** — stamps CI build info, writes schema upgrade notice when bumped, generates offline pnpm sources, builds `org.coloradomesh.MeshClient.flatpak` per arch inside the Flathub freedesktop 24.08 container, smoke-installs the unstamped bundle (manual **Build Flatpak (no release)** dispatch also renames downloadable artifacts to `…-run{N}.flatpak`; tag runs keep clean names) -4. **`publish`** (tag only) — attaches both clean-named `.flatpak` files to the GitHub Release with **`draft: true`** (does not auto-publish an existing draft) +4. **`publish`** (tag only) — waits for the Electron prepare draft (`ci-wait-github-draft-release.mjs`), then attaches both clean-named `.flatpak` files with `ci-upload-release-assets.mjs` using the shared `release_id` (never creates or publishes a release) Both tag-triggered workflows must complete before the release is fully populated. Flatpak bundles often arrive a few minutes after the Electron artifacts. @@ -254,7 +254,7 @@ Follow [Semantic Versioning](https://semver.org/): ### Duplicate draft releases for one tag - Historically caused when parallel `dist:*:publish` / softprops jobs each `POST`ed a draft after a List Releases miss. Current CI: only `prepare-github-release` may create (`MESH_CLIENT_ALLOW_DRAFT_CREATE=1`); builds/Flatpak upload by id; Flatpak waits with `ci-wait-github-draft-release.mjs`. -- **Finalize PATCH 403 (`Resource not accessible by integration`):** Actions `GITHUB_TOKEN` cannot PATCH `target_commitish` when the tagged commit differs in `.github/workflows/` from the default branch. Consolidation skips that field and treats metadata PATCH failures as non-fatal after assets are merged. +- **Finalize PATCH 403 (`Resource not accessible by integration`):** Actions `GITHUB_TOKEN` cannot PATCH `target_commitish` when the tagged commit differs in `.github/workflows/` from the default branch. Consolidation skips that field. After assets are merged, only metadata PATCH **HTTP 403** is non-fatal; any other status should fail the job and be investigated. - **Assets still split (external fork):** `finalize-github-release` merges via `ci-ensure-github-draft-release.mjs`; outside CI run `node scripts/consolidate-github-release-duplicates.mjs --tag vX.Y.Z` (requires `GH_TOKEN`). - **Do not force-move the `v*` tag while a release workflow is in progress.** Retagging starts another run and (with workflow concurrency) cancels the in-flight build; smoke jobs also assume a stable workflow `github.sha`. - **Smoke tests fail with “ref does not point to the expected commit”:** the tag was moved after the workflow started. Re-run failed jobs only after the tag matches the run’s `headSha`, or merge the checkout `ref: ${{ github.sha }}` fix and trigger a fresh tag run. @@ -277,12 +277,12 @@ pnpm run rebuild pnpm run build ``` -Release jobs run `pnpm run rebuild` automatically before `dist:*:publish`. +Release jobs run `pnpm run rebuild` automatically before `dist:*`. ### Flatpak publish did not attach bundles - Confirm `flatpak.yaml` **`publish`** job ran on the tag (not only manual `workflow_dispatch`) -- The publish step uses `draft: true` so it will not promote a draft to live — it only adds files +- Publish waits for the prepare draft, then uploads with `ci-upload-release-assets.mjs` + `RELEASE_ID` (it does not create or publish a release) --- diff --git a/scripts/ci-ensure-github-draft-release.test.mjs b/scripts/ci-ensure-github-draft-release.test.mjs index 29118acae..cb4c3075f 100644 --- a/scripts/ci-ensure-github-draft-release.test.mjs +++ b/scripts/ci-ensure-github-draft-release.test.mjs @@ -408,4 +408,59 @@ describe('consolidateReleases', () => { ); expect(patchBody.target_commitish).toBeUndefined(); }); + + it('fails consolidate when metadata PATCH returns a non-403 error', async () => { + const exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => undefined); + const fetchMock = vi.fn(async (url, init) => { + const method = init?.method ?? 'GET'; + const href = String(url); + if (method === 'GET' && href.includes('/releases?')) { + return new Response( + JSON.stringify([ + { + id: 1, + tag_name: TAG, + name: '5.21.0', + draft: true, + body: '', + assets: [ + { id: 101, name: 'a' }, + { id: 103, name: 'c' }, + ], + }, + { + id: 2, + tag_name: TAG, + name: '5.21.0', + draft: true, + body: 'dup', + assets: [{ id: 102, name: 'b' }], + }, + ]), + { status: 200 }, + ); + } + if (method === 'GET' && href.endsWith('/releases/assets/102')) { + return new Response(new Uint8Array([1]), { status: 200 }); + } + if (method === 'POST' && href.includes('/releases/1/assets')) { + return new Response(JSON.stringify({ id: 999, name: 'b' }), { status: 201 }); + } + if ( + method === 'DELETE' && + (href.endsWith('/releases/assets/102') || href.endsWith('/releases/2')) + ) { + return new Response('', { status: 200 }); + } + if (method === 'PATCH' && href.endsWith('/releases/1')) { + return new Response(JSON.stringify({ message: 'Server Error' }), { status: 500 }); + } + throw new Error(`Unexpected fetch ${method} ${href}`); + }); + vi.stubGlobal('fetch', fetchMock); + + await consolidateReleases({ tag: TAG, token: 'token', log: () => {} }); + expect(exitSpy).toHaveBeenCalledWith(1); + exitSpy.mockRestore(); + }); }); diff --git a/scripts/ci-patch-draft-release-schema-note.mjs b/scripts/ci-patch-draft-release-schema-note.mjs index 570c40314..8b32c3294 100644 --- a/scripts/ci-patch-draft-release-schema-note.mjs +++ b/scripts/ci-patch-draft-release-schema-note.mjs @@ -113,7 +113,7 @@ export async function patchDraftReleaseSchemaNote(opts) { // Prefer prepare's release_id — avoids List Releases lag right after create. draft = await getReleaseById(opts.releaseId, opts.token); if (draft.draft !== true) { - throw new Error(`No release found for ${opts.tag}`); + throw new Error(`Release ${opts.releaseId} for ${opts.tag} is not a draft`); } } else { const ensured = await ensureDraft({ diff --git a/scripts/ci-patch-draft-release-schema-note.test.mjs b/scripts/ci-patch-draft-release-schema-note.test.mjs index c9ac26363..949c83e78 100644 --- a/scripts/ci-patch-draft-release-schema-note.test.mjs +++ b/scripts/ci-patch-draft-release-schema-note.test.mjs @@ -117,6 +117,27 @@ describe('patchDraftReleaseSchemaNote', () => { expect(patch.mock.calls[0][2].body).toContain('Draft release for v1.0.0.'); }); + it('rejects RELEASE_ID when the release is published', async () => { + const patch = vi.fn(); + await expect( + patchDraftReleaseSchemaNote({ + tag: 'v1.0.0', + token: 'token', + markdown: '# Schema bumped', + releaseId: 2, + ensureDraft: vi.fn(), + listReleases: vi.fn(), + getReleaseById: vi.fn().mockResolvedValue({ + id: 2, + draft: false, + body: 'published', + }), + patch, + }), + ).rejects.toThrow('Release 2 for v1.0.0 is not a draft'); + expect(patch).not.toHaveBeenCalled(); + }); + it('falls back to list when ensure returns a non-draft', async () => { const patch = vi.fn().mockResolvedValue({ id: 2 }); await patchDraftReleaseSchemaNote({ diff --git a/scripts/ci-upload-release-assets.mjs b/scripts/ci-upload-release-assets.mjs index fc31ad9ed..8f8a70f52 100644 --- a/scripts/ci-upload-release-assets.mjs +++ b/scripts/ci-upload-release-assets.mjs @@ -41,20 +41,41 @@ export function resolveUploadFiles(patterns, cwd = process.cwd()) { try { if (statSync(abs).isFile()) { files.add(abs); - continue; } } catch { - // catch-no-log-ok missing path checked below via empty matches + // catch-no-log-ok optional glob with no matches — skip } - fail(`No files matched upload pattern: ${pattern}`); + continue; } for (const match of matches) { files.add(match); } } + if (files.size === 0) { + fail(`No files matched upload patterns: ${patterns.join(' ')}`); + } return [...files].sort((a, b) => a.localeCompare(b)); } +/** + * @param {string[]} files + * @returns {string[]} + */ +export function findDuplicateBasenames(files) { + /** @type {Map} */ + const byBase = new Map(); + for (const filePath of files) { + const base = path.basename(filePath); + const list = byBase.get(base) ?? []; + list.push(filePath); + byBase.set(base, list); + } + return [...byBase.entries()] + .filter(([, paths]) => paths.length > 1) + .map(([base]) => base) + .sort((a, b) => a.localeCompare(b)); +} + /** * @param {{ * releaseId: number, @@ -78,6 +99,12 @@ export async function uploadReleaseAssets(opts) { return 0; } + const duplicates = findDuplicateBasenames(opts.files); + if (duplicates.length > 0) { + fail(`Duplicate basename(s) in upload set (refusing to upload): ${duplicates.join(', ')}`); + return 0; + } + /** @type {Array<{ id: number, name: string }>} */ let existingAssets = [...(release.assets ?? [])]; let uploaded = 0; diff --git a/scripts/ci-upload-release-assets.test.mjs b/scripts/ci-upload-release-assets.test.mjs index 95d43878e..e75fdb3c6 100644 --- a/scripts/ci-upload-release-assets.test.mjs +++ b/scripts/ci-upload-release-assets.test.mjs @@ -3,6 +3,7 @@ import { tmpdir } from 'node:os'; import path from 'node:path'; import { afterEach, describe, expect, it, vi } from 'vitest'; import { + findDuplicateBasenames, parseReleaseId, resolveUploadFiles, uploadReleaseAssets, @@ -26,13 +27,29 @@ describe('parseReleaseId', () => { }); describe('resolveUploadFiles', () => { - it('expands globs to absolute files', () => { + it('expands globs to absolute files and skips empty patterns', () => { const dir = mkdtempSync(path.join(tmpdir(), 'mesh-upload-')); writeFileSync(path.join(dir, 'a.deb'), 'a'); writeFileSync(path.join(dir, 'b.rpm'), 'b'); - const files = resolveUploadFiles(['*.deb', '*.rpm'], dir); + const files = resolveUploadFiles(['*.deb', '*.rpm', 'missing-*.yml'], dir); expect(files.map((file) => path.basename(file)).sort()).toEqual(['a.deb', 'b.rpm']); }); + + it('fails only when every pattern yields no files', () => { + const dir = mkdtempSync(path.join(tmpdir(), 'mesh-upload-empty-')); + const exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => undefined); + resolveUploadFiles(['*.deb', 'missing-*.yml'], dir); + expect(exitSpy).toHaveBeenCalledWith(1); + exitSpy.mockRestore(); + }); +}); + +describe('findDuplicateBasenames', () => { + it('reports duplicated basenames from different directories', () => { + expect( + findDuplicateBasenames(['/tmp/a/latest.yml', '/tmp/b/latest.yml', '/tmp/c/other.yml']), + ).toEqual(['latest.yml']); + }); }); describe('uploadReleaseAssets', () => { @@ -52,6 +69,22 @@ describe('uploadReleaseAssets', () => { exitSpy.mockRestore(); }); + it('refuses duplicate basenames before uploading', async () => { + const exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => undefined); + const upload = vi.fn(); + await uploadReleaseAssets({ + releaseId: 9, + token: 'token', + files: ['/tmp/a/latest.yml', '/tmp/b/latest.yml'], + get: async () => ({ id: 9, draft: true, assets: [] }), + upload, + log: () => {}, + }); + expect(exitSpy).toHaveBeenCalledWith(1); + expect(upload).not.toHaveBeenCalled(); + exitSpy.mockRestore(); + }); + it('uploads each file to a draft release', async () => { const uploads = []; const count = await uploadReleaseAssets({ diff --git a/scripts/ci-wait-github-draft-release.mjs b/scripts/ci-wait-github-draft-release.mjs index 96eaf2f60..5e3c8a659 100644 --- a/scripts/ci-wait-github-draft-release.mjs +++ b/scripts/ci-wait-github-draft-release.mjs @@ -3,17 +3,15 @@ * Wait for prepare-github-release to create the draft, then export release_id. * Used by Flatpak publish so it never POSTs a competing draft. */ -import { appendFileSync } from 'node:fs'; import { pathToFileURL } from 'node:url'; +import { writeReleaseIdOutput } from './ci-ensure-github-draft-release.mjs'; import { authToken, resolveTag, waitForGithubDraftRelease } from './github-release-api.mjs'; async function main() { const tag = resolveTag(process.argv.slice(2), process.env); const token = authToken(process.env); const release = await waitForGithubDraftRelease({ tag, token }); - if (typeof process.env.GITHUB_OUTPUT === 'string' && process.env.GITHUB_OUTPUT) { - appendFileSync(process.env.GITHUB_OUTPUT, `release_id=${release.id}\n`, 'utf8'); - } + writeReleaseIdOutput(process.env.GITHUB_OUTPUT, release.id); console.debug(`[ci-wait-github-draft-release] release_id=${release.id}`); } diff --git a/scripts/github-release-api.mjs b/scripts/github-release-api.mjs index 234f3001c..2c5ad8a43 100644 --- a/scripts/github-release-api.mjs +++ b/scripts/github-release-api.mjs @@ -232,14 +232,22 @@ export async function patchReleaseMetadataBestEffort(releaseId, token, patch, lo method: 'PATCH', body: patch, }); - if (!response.ok) { + if (response.ok) { + return json; + } + // Only HTTP 403 is non-fatal after assets are merged (GITHUB_TOKEN cannot retarget + // across workflow diffs). Other statuses still fail the job for investigation. + if (response.status === 403) { log( - `[github-release] PATCH release ${releaseId} failed (${response.status}): ` + + `[github-release] PATCH release ${releaseId} failed (403): ` + `${json?.message ?? response.statusText} — leaving metadata unchanged after asset merge`, ); return null; } - return json; + fail( + `PATCH release ${releaseId} failed (${response.status}): ${json?.message ?? response.statusText}`, + ); + return null; } export async function getRelease(releaseId, token) { diff --git a/src/main/windows-packaging.contract.test.ts b/src/main/windows-packaging.contract.test.ts index 804aec13a..d9809207f 100644 --- a/src/main/windows-packaging.contract.test.ts +++ b/src/main/windows-packaging.contract.test.ts @@ -205,7 +205,7 @@ describe('Windows packaging (contract)', () => { 'utf-8', ); expect(readFileSync(join(REPO_ROOT, 'scripts', 'resolve-release-matrix.mjs'), 'utf-8')).toMatch( - /platform_key:\s*'win'[\s\S]*build_script:\s*'pnpm run dist:win:publish'/, + /platform_key:\s*'win'[\s\S]*build_script:\s*'pnpm run dist:win'/, ); expect(releaseWorkflow).toContain('scripts/resolve-release-matrix.mjs'); expect(releaseWorkflow).toContain(