From 8daca30d3609dbcfcde267a2c0740449465f3e48 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 21 Jul 2026 16:30:51 +0000 Subject: [PATCH 1/2] fix: avoid duplicate draft releases when publishing multiple dry runs When resuming multiple saved dry runs (publish --from-dry-run), the same PublisherGithub instance is asked to publish once per dry run. Each call listed the repository's releases to find the target release, but GitHub's list releases API is eventually consistent, so a release created seconds earlier by the previous call could be missing from the listing - causing a second, duplicate draft release to be created for the same tag and the platform assets to be split across the two drafts. Remember the releases found or created during this process, keyed by tag name, and consult that cache before hitting the list releases endpoint. Fixes #4325 # Conflicts: # packages/publisher/github/src/PublisherGithub.ts --- .../github/spec/PublisherGitHub.spec.ts | 100 ++++++++++++++++++ .../publisher/github/src/PublisherGitHub.ts | 44 +++++--- 2 files changed, 128 insertions(+), 16 deletions(-) create mode 100644 packages/publisher/github/spec/PublisherGitHub.spec.ts diff --git a/packages/publisher/github/spec/PublisherGitHub.spec.ts b/packages/publisher/github/spec/PublisherGitHub.spec.ts new file mode 100644 index 0000000000..ed5e54fb7a --- /dev/null +++ b/packages/publisher/github/spec/PublisherGitHub.spec.ts @@ -0,0 +1,100 @@ +import os from 'node:os'; +import path from 'node:path'; + +import { ForgeMakeResult } from '@electron-forge/shared-types'; +import fs from 'fs-extra'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { PublisherGitHub } from '../src/PublisherGitHub'; + +import type { PublisherOptions } from '@electron-forge/publisher-base'; + +const { mockOctokit } = vi.hoisted(() => ({ + mockOctokit: { + repos: { + listReleases: vi.fn(), + createRelease: vi.fn(), + uploadReleaseAsset: vi.fn(), + }, + }, +})); + +vi.mock('../src/util/github', async (importOriginal) => { + const mod = await importOriginal(); + class MockGitHub extends mod.default { + getGitHub() { + return mockOctokit as unknown as ReturnType< + InstanceType['getGitHub'] + >; + } + } + return { ...mod, default: MockGitHub }; +}); + +describe('PublisherGitHub', () => { + let tmpDir: string; + + const makeResultFor = async ( + artifactName: string, + ): Promise => { + const artifactPath = path.resolve(tmpDir, artifactName); + await fs.writeFile(artifactPath, 'fake-artifact'); + return { + artifacts: [artifactPath], + packageJSON: { version: '1.0.0' }, + platform: 'darwin', + arch: 'x64', + }; + }; + + const publishFor = async ( + publisher: PublisherGitHub, + artifactName: string, + ) => { + await publisher.publish({ + dir: tmpDir, + makeResults: [await makeResultFor(artifactName)], + setStatusLine: vi.fn(), + } as unknown as PublisherOptions); + }; + + beforeEach(async () => { + tmpDir = await fs.mkdtemp( + path.resolve(os.tmpdir(), 'forge-publisher-github-'), + ); + // Simulate GitHub's eventually consistent "list releases" API by never + // returning the release created moments earlier in the same process + mockOctokit.repos.listReleases.mockResolvedValue({ data: [] }); + mockOctokit.repos.createRelease.mockResolvedValue({ + data: { + id: 123, + tag_name: 'v1.0.0', + upload_url: 'https://example.com/upload', + assets: [], + }, + }); + mockOctokit.repos.uploadReleaseAsset.mockImplementation( + async ({ name }: { name: string }) => ({ data: { name } }), + ); + }); + + afterEach(async () => { + await fs.remove(tmpDir); + }); + + it('does not create a duplicate release when publishing multiple dry runs for the same version', async () => { + const publisher = new PublisherGitHub({ + repository: { owner: 'my-owner', name: 'my-repo' }, + draft: true, + }); + + await publishFor(publisher, 'app-1.0.0-darwin.zip'); + await publishFor(publisher, 'app-1.0.0-win32.zip'); + + expect(mockOctokit.repos.createRelease).toHaveBeenCalledOnce(); + expect(mockOctokit.repos.uploadReleaseAsset).toHaveBeenCalledTimes(2); + for (const [args] of mockOctokit.repos.uploadReleaseAsset.mock.calls) { + expect(args.release_id).toEqual(123); + } + }); +}); diff --git a/packages/publisher/github/src/PublisherGitHub.ts b/packages/publisher/github/src/PublisherGitHub.ts index 1f4a47235d..a4d232c04e 100644 --- a/packages/publisher/github/src/PublisherGitHub.ts +++ b/packages/publisher/github/src/PublisherGitHub.ts @@ -27,6 +27,13 @@ interface GitHubRelease { upload_url: string; } +type OctokitRelease = GetResponseDataTypeFromEndpointMethod< + Octokit['repos']['getRelease'] +>; +type OctokitReleaseAsset = GetResponseDataTypeFromEndpointMethod< + Octokit['repos']['updateReleaseAsset'] +>; + // Streams a file as the request body so that byte-level upload progress can be // tracked. Octokit passes the body straight to `fetch`, which streams a // ReadableStream body (with `duplex: 'half'`), invoking `onProgress` with the @@ -58,6 +65,12 @@ function progressStream( export default class PublisherGitHub extends PublisherBase { name = 'github'; + // Releases we already found or created in this process, keyed by tag name. + // GitHub's "list releases" API is eventually consistent, so a release + // created moments ago (e.g. by a previous restored dry run) may be missing + // from the listing, which would cause a duplicate release to be created. + private knownReleases = new Map(); + async publish({ makeResults, setStatusLine, @@ -92,13 +105,6 @@ export default class PublisherGitHub extends PublisherBase; - type OctokitReleaseAsset = GetResponseDataTypeFromEndpointMethod< - Octokit['repos']['updateReleaseAsset'] - >; - for (const releaseVersion of Object.keys(perReleaseArtifacts)) { let release: OctokitRelease | undefined; const artifacts = perReleaseArtifacts[releaseVersion]; @@ -106,15 +112,18 @@ export default class PublisherGitHub extends PublisherBase testRelease.tag_name === releaseName, - ); + release = + this.knownReleases.get(releaseName) ?? + ( + await github.getGitHub().repos.listReleases({ + owner: config.repository.owner, + repo: config.repository.name, + per_page: 100, + }) + ).data.find( + (testRelease: GitHubRelease) => + testRelease.tag_name === releaseName, + ); if (!release) { throw new NoReleaseError(404); } @@ -138,6 +147,9 @@ export default class PublisherGitHub extends PublisherBase artifact.artifacts); const artifactSizes = await Promise.all( artifactPaths.map(async (p) => (await fs.stat(p)).size), From b70125fb89e0a7e4aa7b23207a0e0fcfa7edd2c9 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 21 Jul 2026 19:42:33 +0000 Subject: [PATCH 2/2] test: use node:fs and explicit auth token in PublisherGitHub spec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The spec imported fs-extra, which is not a dependency of the publisher-github package on this branch (knip failed CI with an unlisted-dependency error); use node:fs/promises instead. Also stub GITHUB_TOKEN and pass an explicit authToken in the publisher config so the spec does not depend on ambient credentials — CI runners have no GITHUB_TOKEN and the GitHub client constructor throws without either. Mirrors the equivalent fix on the main-branch PR. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01VpZV6TdUUfbWBHduegyqgc --- packages/publisher/github/spec/PublisherGitHub.spec.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/publisher/github/spec/PublisherGitHub.spec.ts b/packages/publisher/github/spec/PublisherGitHub.spec.ts index ed5e54fb7a..e412d65945 100644 --- a/packages/publisher/github/spec/PublisherGitHub.spec.ts +++ b/packages/publisher/github/spec/PublisherGitHub.spec.ts @@ -1,8 +1,8 @@ +import fs from 'node:fs/promises'; import os from 'node:os'; import path from 'node:path'; import { ForgeMakeResult } from '@electron-forge/shared-types'; -import fs from 'fs-extra'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { PublisherGitHub } from '../src/PublisherGitHub'; @@ -59,6 +59,9 @@ describe('PublisherGitHub', () => { }; beforeEach(async () => { + // Ensure the spec does not depend on ambient credentials — CI runners + // have no GITHUB_TOKEN, so the publisher must rely on config.authToken + vi.stubEnv('GITHUB_TOKEN', ''); tmpDir = await fs.mkdtemp( path.resolve(os.tmpdir(), 'forge-publisher-github-'), ); @@ -79,13 +82,15 @@ describe('PublisherGitHub', () => { }); afterEach(async () => { - await fs.remove(tmpDir); + vi.unstubAllEnvs(); + await fs.rm(tmpDir, { recursive: true, force: true }); }); it('does not create a duplicate release when publishing multiple dry runs for the same version', async () => { const publisher = new PublisherGitHub({ repository: { owner: 'my-owner', name: 'my-repo' }, draft: true, + authToken: 'fake-token', }); await publishFor(publisher, 'app-1.0.0-darwin.zip');