From 21892c5f6304f8057ecf59153e6f02fe76e37fab Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Mon, 29 Dec 2025 18:28:10 +0300 Subject: [PATCH] fix(github): Clean up orphaned draft releases on publish failure When a publish action fails after creating a draft release, the draft would be left orphaned on GitHub. This caused multiple draft releases with the same tag name to accumulate when re-running releases. Changes: - Add deleteRelease method to GitHubTarget that only deletes draft releases - Wrap publish flow in try-catch to clean up drafts on failure - Apply same cleanup pattern to UPM target - Add tests for draft release cleanup behavior Fixes #388 --- src/targets/__tests__/github.test.ts | 165 ++++++++++++++++++++++++++- src/targets/github.ts | 64 ++++++++++- src/targets/upm.ts | 21 +++- 3 files changed, 240 insertions(+), 10 deletions(-) diff --git a/src/targets/__tests__/github.test.ts b/src/targets/__tests__/github.test.ts index 2a1d1a13..0cfb16d6 100644 --- a/src/targets/__tests__/github.test.ts +++ b/src/targets/__tests__/github.test.ts @@ -1,5 +1,7 @@ import { vi, type Mock, type MockInstance, type Mocked, type MockedFunction } from 'vitest'; -import { isLatestRelease } from '../github'; +import { isLatestRelease, GitHubTarget } from '../github'; +import { NoneArtifactProvider } from '../../artifact_providers/none'; +import { setGlobals } from '../../utils/helpers'; describe('isLatestRelease', () => { it('works with missing latest release', () => { @@ -62,3 +64,164 @@ describe('isLatestRelease', () => { }); }); }); + +describe('GitHubTarget', () => { + const cleanEnv = { ...process.env }; + let githubTarget: GitHubTarget; + + beforeEach(() => { + process.env = { + ...cleanEnv, + GITHUB_TOKEN: 'test github token', + }; + setGlobals({ 'dry-run': false, 'log-level': 'Info', 'no-input': true }); + vi.resetAllMocks(); + + githubTarget = new GitHubTarget( + { name: 'github' }, + new NoneArtifactProvider(), + { owner: 'testOwner', repo: 'testRepo' } + ); + }); + + afterEach(() => { + process.env = cleanEnv; + }); + + describe('publish', () => { + const mockDraftRelease = { + id: 123, + tag_name: 'v1.0.0', + upload_url: 'https://example.com/upload', + draft: true, + }; + + beforeEach(() => { + // Mock all the methods that publish depends on + githubTarget.getArtifactsForRevision = vi.fn().mockResolvedValue([]); + githubTarget.createDraftRelease = vi + .fn() + .mockResolvedValue(mockDraftRelease); + githubTarget.deleteRelease = vi.fn().mockResolvedValue(true); + githubTarget.publishRelease = vi.fn().mockResolvedValue(undefined); + githubTarget.github.repos.getLatestRelease = vi.fn().mockRejectedValue({ + status: 404, + }); + }); + + it('cleans up draft release when publishRelease fails', async () => { + const publishError = new Error('Publish failed'); + githubTarget.publishRelease = vi.fn().mockRejectedValue(publishError); + + await expect( + githubTarget.publish('1.0.0', 'abc123') + ).rejects.toThrow('Publish failed'); + + expect(githubTarget.deleteRelease).toHaveBeenCalledWith(mockDraftRelease); + }); + + it('still throws original error if deleteRelease also fails', async () => { + const publishError = new Error('Publish failed'); + const deleteError = new Error('Delete failed'); + + githubTarget.publishRelease = vi.fn().mockRejectedValue(publishError); + githubTarget.deleteRelease = vi.fn().mockRejectedValue(deleteError); + + await expect( + githubTarget.publish('1.0.0', 'abc123') + ).rejects.toThrow('Publish failed'); + + expect(githubTarget.deleteRelease).toHaveBeenCalledWith(mockDraftRelease); + }); + + it('does not delete release when publish succeeds', async () => { + await githubTarget.publish('1.0.0', 'abc123'); + + expect(githubTarget.deleteRelease).not.toHaveBeenCalled(); + }); + }); + + describe('deleteRelease', () => { + it('deletes a draft release', async () => { + const draftRelease = { + id: 123, + tag_name: 'v1.0.0', + upload_url: 'https://example.com/upload', + draft: true, + }; + + githubTarget.github.repos.deleteRelease = vi + .fn() + .mockResolvedValue({ status: 204 }); + + const result = await githubTarget.deleteRelease(draftRelease); + + expect(result).toBe(true); + expect(githubTarget.github.repos.deleteRelease).toHaveBeenCalledWith({ + release_id: 123, + owner: 'testOwner', + repo: 'testRepo', + changelog: 'CHANGELOG.md', + previewReleases: true, + tagPrefix: '', + tagOnly: false, + floatingTags: [], + }); + }); + + it('refuses to delete a non-draft release', async () => { + const publishedRelease = { + id: 123, + tag_name: 'v1.0.0', + upload_url: 'https://example.com/upload', + draft: false, + }; + + githubTarget.github.repos.deleteRelease = vi + .fn() + .mockResolvedValue({ status: 204 }); + + const result = await githubTarget.deleteRelease(publishedRelease); + + expect(result).toBe(false); + expect(githubTarget.github.repos.deleteRelease).not.toHaveBeenCalled(); + }); + + it('allows deletion when draft status is undefined (backwards compatibility)', async () => { + const releaseWithoutDraftFlag = { + id: 123, + tag_name: 'v1.0.0', + upload_url: 'https://example.com/upload', + }; + + githubTarget.github.repos.deleteRelease = vi + .fn() + .mockResolvedValue({ status: 204 }); + + const result = await githubTarget.deleteRelease(releaseWithoutDraftFlag); + + expect(result).toBe(true); + expect(githubTarget.github.repos.deleteRelease).toHaveBeenCalled(); + }); + + it('does not delete in dry-run mode', async () => { + setGlobals({ 'dry-run': true, 'log-level': 'Info', 'no-input': true }); + + const draftRelease = { + id: 123, + tag_name: 'v1.0.0', + upload_url: 'https://example.com/upload', + draft: true, + }; + + githubTarget.github.repos.deleteRelease = vi + .fn() + .mockResolvedValue({ status: 204 }); + + const result = await githubTarget.deleteRelease(draftRelease); + + expect(result).toBe(false); + expect(githubTarget.github.repos.deleteRelease).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/src/targets/github.ts b/src/targets/github.ts index d4103bc5..089b6e7b 100644 --- a/src/targets/github.ts +++ b/src/targets/github.ts @@ -62,6 +62,8 @@ interface GitHubRelease { tag_name: string; /** Upload URL */ upload_url: string; + /** Whether this is a draft release */ + draft?: boolean; } type ReposListAssetsForReleaseResponseItem = RestEndpointMethodTypes['repos']['listReleaseAssets']['response']['data'][0]; @@ -136,6 +138,7 @@ export class GitHubTarget extends BaseTarget { id: 0, tag_name: tag, upload_url: '', + draft: true, }; } @@ -197,6 +200,40 @@ export class GitHubTarget extends BaseTarget { ); } + /** + * Deletes the provided release if it is a draft + * + * Used to clean up orphaned draft releases when publish fails. + * Refuses to delete non-draft releases as a safety measure. + * + * @param release Release to delete + * @returns true if deleted, false if skipped (dry-run or not a draft) + */ + public async deleteRelease(release: GitHubRelease): Promise { + this.logger.debug(`Deleting release: "${release.tag_name}"...`); + + if (release.draft === false) { + this.logger.warn( + `Refusing to delete release "${release.tag_name}" because it is not a draft` + ); + return false; + } + + if (isDryRun()) { + this.logger.info(`[dry-run] Not deleting release "${release.tag_name}"`); + return false; + } + + return ( + ( + await this.github.repos.deleteRelease({ + release_id: release.id, + ...this.githubConfig, + }) + ).status === 204 + ); + } + /** * Fetches a list of all assets for the given release * @@ -532,13 +569,28 @@ export class GitHubTarget extends BaseTarget { changelog ); - await Promise.all( - localArtifacts.map(({ path, mimeType }) => - this.uploadAsset(draftRelease, path, mimeType) - ) - ); + try { + await Promise.all( + localArtifacts.map(({ path, mimeType }) => + this.uploadAsset(draftRelease, path, mimeType) + ) + ); - await this.publishRelease(draftRelease, { makeLatest }); + await this.publishRelease(draftRelease, { makeLatest }); + } catch (error) { + // Clean up the orphaned draft release + try { + await this.deleteRelease(draftRelease); + this.logger.info( + `Deleted orphaned draft release: ${draftRelease.tag_name}` + ); + } catch (deleteError) { + this.logger.warn( + `Failed to delete orphaned draft release: ${deleteError}` + ); + } + throw error; + } // Update floating tags (e.g., v2 for version 2.15.0) await this.updateFloatingTags(version, revision); diff --git a/src/targets/upm.ts b/src/targets/upm.ts index 927eda6e..1529aaa2 100644 --- a/src/targets/upm.ts +++ b/src/targets/upm.ts @@ -147,9 +147,24 @@ export class UpmTarget extends BaseTarget { targetRevision, changes ); - await this.githubTarget.publishRelease(draftRelease, { - makeLatest: !isPrerelease, - }); + try { + await this.githubTarget.publishRelease(draftRelease, { + makeLatest: !isPrerelease, + }); + } catch (error) { + // Clean up the orphaned draft release + try { + await this.githubTarget.deleteRelease(draftRelease); + this.logger.info( + `Deleted orphaned draft release: ${draftRelease.tag_name}` + ); + } catch (deleteError) { + this.logger.warn( + `Failed to delete orphaned draft release: ${deleteError}` + ); + } + throw error; + } } }, true,