fix: publish existing draft releases as prereleases (#801)

* fix: respect input_draft when updating existing release * fix: reuse draft releases before publishing Signed-off-by: Rui Chen <rui@chenrui.dev> * fix: bound and canonicalize draft discovery Signed-off-by: Rui Chen <rui@chenrui.dev> * fix: preserve unowned draft releases Signed-off-by: Rui Chen <rui@chenrui.dev> * fix: recheck duplicate drafts before cleanup Signed-off-by: Rui Chen <rui@chenrui.dev> --------- Signed-off-by: Rui Chen <rui@chenrui.dev> Co-authored-by: Rui Chen <rui@chenrui.dev>
2026-07-15 23:13:02 +08:00 · 2026-07-13 13:59:29 +08:00
parent d8a89a2066
commit f345337888
5 changed files with 427 additions and 83 deletions
@@ -130,6 +130,92 @@ describe('github', () => {
      assert.deepStrictEqual(result, targetRelease);
    });

+    it('falls back to paginated releases when the direct lookup cannot see a draft', async () => {
+      const draftRelease = { ...mockRelease, draft: true };
+      const pageAfterMatch = vi.fn();
+      const releaser = {
+        ...mockReleaser,
+        allReleases: async function* () {
+          yield { data: [{ ...mockRelease, tag_name: 'other' }] };
+          yield { data: [draftRelease] };
+          pageAfterMatch();
+          yield { data: [] };
+        },
+      };
+
+      const result = await findTagFromReleases(releaser, owner, repo, draftRelease.tag_name);
+
+      assert.strictEqual(result, draftRelease);
+      expect(pageAfterMatch).not.toHaveBeenCalled();
+    });
+
+    it('does not exhaust pagination while checking a brand-new tag', async () => {
+      let pagesRead = 0;
+      const releaser = {
+        ...mockReleaser,
+        allReleases: async function* () {
+          pagesRead += 1;
+          yield { data: [] };
+          pagesRead += 1;
+          yield { data: [] };
+          throw new Error('Only the first 10000 results are available');
+        },
+      };
+
+      const result = await findTagFromReleases(releaser, owner, repo, 'brand-new-tag');
+
+      expect(result).toBeUndefined();
+      expect(pagesRead).toBe(2);
+    });
+
+    it('selects the canonical draft without deleting a pre-existing duplicate', async () => {
+      const canonicalDraft: Release = {
+        ...mockRelease,
+        id: 1,
+        draft: true,
+        assets: [{ id: 99, name: 'existing.zip' }],
+      };
+      const duplicateDraft: Release = {
+        ...canonicalDraft,
+        id: 2,
+        name: 'manually authored draft',
+        body: 'notes that must not be deleted',
+        assets: [],
+      };
+      const deleteRelease = vi.fn().mockResolvedValue(undefined);
+      const releaser = {
+        ...mockReleaser,
+        allReleases: async function* () {
+          yield { data: [duplicateDraft, canonicalDraft] };
+        },
+        deleteRelease,
+      };
+
+      const result = await findTagFromReleases(releaser, owner, repo, canonicalDraft.tag_name);
+
+      expect(result).toBe(canonicalDraft);
+      expect(deleteRelease).not.toHaveBeenCalled();
+    });
+
+    it('retries draft discovery when GitHub release listing is briefly stale', async () => {
+      vi.useFakeTimers();
+      const draftRelease = { ...mockRelease, draft: true };
+      let listingAttempt = 0;
+      const releaser = {
+        ...mockReleaser,
+        allReleases: async function* () {
+          listingAttempt += 1;
+          yield { data: listingAttempt === 1 ? [] : [draftRelease] };
+        },
+      };
+
+      const resultPromise = findTagFromReleases(releaser, owner, repo, draftRelease.tag_name, 2);
+      await vi.advanceTimersByTimeAsync(1000);
+
+      await expect(resultPromise).resolves.toBe(draftRelease);
+      expect(listingAttempt).toBe(2);
+    });
+
    it('returns undefined when release is not found (404)', async () => {
      const releaser = {
        ...mockReleaser,
@@ -155,6 +241,18 @@ describe('github', () => {
      }
    });

+    it('re-throws errors from the draft listing fallback', async () => {
+      const listingError = new Error('release listing failed');
+      const releaser = {
+        ...mockReleaser,
+        allReleases: async function* () {
+          throw listingError;
+        },
+      };
+
+      await expect(findTagFromReleases(releaser, owner, repo, 'v1.0.0')).rejects.toBe(listingError);
+    });
+
    it('finds a release with empty tag name', async () => {
      const emptyTag = '';
      const targetRelease = {
@@ -603,6 +701,9 @@ describe('github', () => {
      const releaser = createReleaser({
        getReleaseByTag: vi.fn().mockRejectedValue({ status: 404 }),
        createRelease: vi.fn().mockRejectedValue(releaseError),
+        allReleases: async function* () {
+          yield { data: [] };
+        },
      });

      await expect(release(config, releaser, 1)).rejects.toBe(releaseError);
@@ -725,14 +826,18 @@ describe('github', () => {
        assets: [],
      };

-      const createReleaseSpy = vi.fn(async () => ({ data: createdRelease }));
+      let created = false;
+      const createReleaseSpy = vi.fn(async () => {
+        created = true;
+        return { data: createdRelease };
+      });
      const mockReleaser: Releaser = {
        getReleaseByTag: () => Promise.reject({ status: 404 }),
        createRelease: createReleaseSpy,
        updateRelease: () => Promise.reject('Not implemented'),
        finalizeRelease: () => Promise.reject('Not implemented'),
        allReleases: async function* () {
-          yield { data: [createdRelease] };
+          yield { data: created ? [createdRelease] : [] };
        },
        listReleaseAssets: () => Promise.reject('Not implemented'),
        deleteReleaseAsset: () => Promise.reject('Not implemented'),
@@ -753,6 +858,59 @@ describe('github', () => {
      );
    });

+    it.each([
+      ['an omitted draft input', undefined],
+      ['a null-expression draft input', undefined],
+      ['an explicit false draft input', false],
+      ['an explicit true draft input', true],
+    ])('reuses an existing draft for %s', async (_name, input_draft) => {
+      const existingDraft: Release = {
+        id: 17,
+        upload_url: 'draft-upload',
+        html_url: 'draft-html',
+        tag_name: 'v1.0.0',
+        name: 'draft release',
+        body: 'draft body',
+        target_commitish: 'main',
+        draft: true,
+        prerelease: false,
+        assets: [],
+      };
+      const updatedDraft = { ...existingDraft, prerelease: true };
+      const updateRelease = vi.fn().mockResolvedValue({ data: updatedDraft });
+      const createRelease = unexpected('createRelease');
+      const deleteRelease = unexpected('deleteRelease');
+      const releaser = createReleaser({
+        getReleaseByTag: vi.fn().mockRejectedValue({ status: 404 }),
+        allReleases: async function* () {
+          yield { data: [existingDraft] };
+        },
+        createRelease,
+        updateRelease,
+        deleteRelease,
+      });
+
+      const result = await release(
+        {
+          ...config,
+          input_draft,
+          input_prerelease: true,
+        },
+        releaser,
+      );
+
+      expect(result).toEqual({ release: updatedDraft, created: false });
+      expect(updateRelease).toHaveBeenCalledWith(
+        expect.objectContaining({
+          release_id: existingDraft.id,
+          draft: true,
+          prerelease: true,
+        }),
+      );
+      expect(createRelease).not.toHaveBeenCalled();
+      expect(deleteRelease).not.toHaveBeenCalled();
+    });
+
    it('retries upload after deleting conflicting asset on 422 already_exists race', async () => {
      const uploadReleaseAsset = vi
        .fn()
@@ -923,6 +1081,7 @@ describe('github', () => {
    });

    it('handles 422 already_exists error gracefully', async () => {
+      vi.useFakeTimers();
      const existingRelease = {
        id: 1,
        upload_url: 'test',
@@ -969,7 +1128,7 @@ describe('github', () => {
          }),
        finalizeRelease: () => Promise.reject('Not implemented'),
        allReleases: async function* () {
-          yield { data: [existingRelease] };
+          yield { data: createAttempts > 0 ? [existingRelease] : [] };
        },
        listReleaseAssets: () => Promise.reject('Not implemented'),
        deleteReleaseAsset: () => Promise.reject('Not implemented'),
@@ -977,7 +1136,9 @@ describe('github', () => {
        uploadReleaseAsset: () => Promise.reject('Not implemented'),
      } as const;

-      const result = await release(config, mockReleaser, 2);
+      const resultPromise = release(config, mockReleaser, 2);
+      await vi.advanceTimersByTimeAsync(1000);
+      const result = await resultPromise;
      assert.ok(result);
      assert.equal(result.release.id, 1);
      assert.equal(result.created, false);
@@ -1071,11 +1232,14 @@ describe('github', () => {
          }
          return Promise.resolve({ data: canonicalRelease });
        },
-        createRelease: () => Promise.resolve({ data: duplicateRelease }),
+        createRelease: () => {
+          lookupCount += 1;
+          return Promise.resolve({ data: duplicateRelease });
+        },
        updateRelease: () => Promise.reject('Not implemented'),
        finalizeRelease: () => Promise.reject('Not implemented'),
        allReleases: async function* () {
-          yield { data: [duplicateRelease, canonicalRelease] };
+          yield { data: lookupCount > 1 ? [duplicateRelease, canonicalRelease] : [] };
        },
        listReleaseAssets: () => Promise.reject('Not implemented'),
        deleteReleaseAsset: () => Promise.reject('Not implemented'),
@@ -1084,7 +1248,7 @@ describe('github', () => {
        uploadReleaseAsset: () => Promise.reject('Not implemented'),
      };

-      const result = await release(config, mockReleaser, 2);
+      const result = await release(config, mockReleaser, 1);

      assert.equal(result.release.id, canonicalRelease.id);
      assert.equal(result.created, false);
@@ -1095,6 +1259,47 @@ describe('github', () => {
      });
    });

+    it('does not delete a pre-existing draft while canonicalizing its own create', async () => {
+      const createdRelease: Release = {
+        id: 1,
+        upload_url: 'created-upload',
+        html_url: 'created-html',
+        tag_name: 'v1.0.0',
+        name: 'created release',
+        body: 'created body',
+        target_commitish: 'main',
+        draft: true,
+        prerelease: false,
+        assets: [],
+      };
+      const manualDraft: Release = {
+        ...createdRelease,
+        id: 2,
+        name: 'manually authored draft',
+        body: 'notes that must not be deleted',
+      };
+      let created = false;
+      const deleteRelease = vi.fn().mockResolvedValue(undefined);
+      const releaser = createReleaser({
+        getReleaseByTag: vi.fn(() =>
+          created ? Promise.resolve({ data: createdRelease }) : Promise.reject({ status: 404 }),
+        ),
+        createRelease: vi.fn(async () => {
+          created = true;
+          return { data: createdRelease };
+        }),
+        allReleases: async function* () {
+          yield { data: created ? [createdRelease, manualDraft] : [] };
+        },
+        deleteRelease,
+      });
+
+      const result = await release(config, releaser, 1);
+
+      expect(result).toEqual({ release: createdRelease, created: true });
+      expect(deleteRelease).not.toHaveBeenCalled();
+    });
+
    it('falls back to recent releases when tag lookup still lags after create', async () => {
      const canonicalRelease: Release = {
        id: 1,
@@ -1121,14 +1326,18 @@ describe('github', () => {
        assets: [],
      };

+      let created = false;
      const deleteReleaseSpy = vi.fn(async () => undefined);
      const mockReleaser: Releaser = {
        getReleaseByTag: () => Promise.reject({ status: 404 }),
-        createRelease: () => Promise.resolve({ data: duplicateRelease }),
+        createRelease: () => {
+          created = true;
+          return Promise.resolve({ data: duplicateRelease });
+        },
        updateRelease: () => Promise.reject('Not implemented'),
        finalizeRelease: () => Promise.reject('Not implemented'),
        allReleases: async function* () {
-          yield { data: [duplicateRelease, canonicalRelease] };
+          yield { data: created ? [duplicateRelease, canonicalRelease] : [] };
        },
        listReleaseAssets: () => Promise.reject('Not implemented'),
        deleteReleaseAsset: () => Promise.reject('Not implemented'),
@@ -1148,7 +1357,59 @@ describe('github', () => {
      });
    });

-    it('deletes the just-created duplicate draft even if recent release listing misses it', async () => {
+    it.each([
+      ['gains assets', { assets: [{ id: 9, name: 'concurrent.zip' }] }],
+      ['is published', { draft: false }],
+    ])('does not delete its created release when it %s concurrently', async (_name, patch) => {
+      const canonicalRelease: Release = {
+        id: 1,
+        upload_url: 'canonical-upload',
+        html_url: 'canonical-html',
+        tag_name: 'v1.0.0',
+        name: 'canonical',
+        body: 'test',
+        target_commitish: 'main',
+        draft: true,
+        prerelease: false,
+        assets: [],
+      };
+      const createdRelease: Release = {
+        ...canonicalRelease,
+        id: 2,
+        name: 'created duplicate',
+      };
+      const refreshedCreatedRelease: Release = { ...createdRelease, ...patch };
+      const releaseByTag = refreshedCreatedRelease.draft
+        ? canonicalRelease
+        : refreshedCreatedRelease;
+      let created = false;
+      const deleteRelease = vi.fn().mockResolvedValue(undefined);
+      const releaser = createReleaser({
+        getReleaseByTag: vi.fn(() =>
+          created ? Promise.resolve({ data: releaseByTag }) : Promise.reject({ status: 404 }),
+        ),
+        createRelease: vi.fn(async () => {
+          created = true;
+          return { data: createdRelease };
+        }),
+        allReleases: async function* () {
+          yield {
+            data: created ? [refreshedCreatedRelease, canonicalRelease] : [],
+          };
+        },
+        deleteRelease,
+      });
+
+      const result = await release(config, releaser, 1);
+
+      expect(result).toEqual({
+        release: releaseByTag,
+        created: releaseByTag.id === createdRelease.id,
+      });
+      expect(deleteRelease).not.toHaveBeenCalled();
+    });
+
+    it('does not delete the just-created duplicate when refreshed state is unavailable', async () => {
      const canonicalRelease: Release = {
        id: 1,
        upload_url: 'canonical-upload',
@@ -1184,11 +1445,14 @@ describe('github', () => {
          }
          return Promise.resolve({ data: canonicalRelease });
        },
-        createRelease: () => Promise.resolve({ data: duplicateRelease }),
+        createRelease: () => {
+          lookupCount += 1;
+          return Promise.resolve({ data: duplicateRelease });
+        },
        updateRelease: () => Promise.reject('Not implemented'),
        finalizeRelease: () => Promise.reject('Not implemented'),
        allReleases: async function* () {
-          yield { data: [canonicalRelease] };
+          yield { data: lookupCount > 1 ? [canonicalRelease] : [] };
        },
        listReleaseAssets: () => Promise.reject('Not implemented'),
        deleteReleaseAsset: () => Promise.reject('Not implemented'),
@@ -1197,15 +1461,11 @@ describe('github', () => {
        uploadReleaseAsset: () => Promise.reject('Not implemented'),
      };

-      const result = await release(config, mockReleaser, 2);
+      const result = await release(config, mockReleaser, 1);

      assert.equal(result.release.id, canonicalRelease.id);
      assert.equal(result.created, false);
-      expect(deleteReleaseSpy).toHaveBeenCalledWith({
-        owner: 'owner',
-        repo: 'repo',
-        release_id: duplicateRelease.id,
-      });
+      expect(deleteReleaseSpy).not.toHaveBeenCalled();
    });
  });

@@ -256,6 +256,52 @@ describe('run', () => {
    await result;
  });

+  it('keeps an existing draft unpublished until its upload completes', async () => {
+    config = {
+      ...config,
+      input_draft: false,
+      input_prerelease: true,
+      input_files: ['asset.zip'],
+    };
+    mocks.release.mockResolvedValue({ release: initialRelease, created: false });
+    mocks.paths.mockReturnValue(['asset.zip']);
+    const pendingUpload = deferred<{ id: number }>();
+    mocks.upload.mockReturnValue(pendingUpload.promise);
+
+    const result = run();
+    await Promise.resolve();
+
+    expect(mocks.upload).toHaveBeenCalledOnce();
+    expect(mocks.finalizeRelease).not.toHaveBeenCalled();
+
+    pendingUpload.resolve({ id: 7 });
+    await result;
+
+    expect(mocks.finalizeRelease).toHaveBeenCalledWith(
+      config,
+      mocks.releaser,
+      initialRelease,
+      false,
+    );
+  });
+
+  it('leaves an existing draft recoverable when an upload fails', async () => {
+    config = {
+      ...config,
+      input_draft: false,
+      input_prerelease: true,
+      input_files: ['asset.zip'],
+    };
+    mocks.release.mockResolvedValue({ release: initialRelease, created: false });
+    mocks.paths.mockReturnValue(['asset.zip']);
+    mocks.upload.mockRejectedValue(new Error('upload failed'));
+
+    await run();
+
+    expect(mocks.finalizeRelease).not.toHaveBeenCalled();
+    expect(mocks.setFailed).toHaveBeenCalledWith('upload failed');
+  });
+
  it('finalizes after uploads and outputs only newly uploaded assets without uploader data', async () => {
    config = { ...config, input_files: ['one.zip', 'skipped.zip', 'two.zip'] };
    mocks.paths.mockReturnValue(['one.zip', 'skipped.zip', 'two.zip']);
@@ -234,6 +234,10 @@ describe('util', () => {
      );
    });

+    it('treats an empty draft input as omitted', () => {
+      assert.strictEqual(parseConfig({ INPUT_DRAFT: '' }).input_draft, undefined);
+    });
+
    it('parses basic config with commitish', () => {
      assert.deepStrictEqual(
        parseConfig({
@@ -531,23 +531,31 @@ export const release = async (
  if (generate_release_notes && previous_tag_name) {
    console.log(`📝 Generating release notes using previous tag ${previous_tag_name}`);
  }
+  let _release: Release | undefined;
  try {
-    const _release: Release | undefined = await findTagFromReleases(releaser, owner, repo, tag);
+    _release = await findTagFromReleases(releaser, owner, repo, tag, maxRetries);
+  } catch (error) {
+    console.log(
+      `⚠️ Unexpected error fetching GitHub release for tag ${config.github_ref}: ${error}`,
+    );
+    throw error;
+  }

-    if (_release === undefined) {
-      return await createRelease(
-        tag,
-        config,
-        releaser,
-        owner,
-        repo,
-        discussion_category_name,
-        generate_release_notes,
-        maxRetries,
-        previous_tag_name,
-      );
-    }
+  if (_release === undefined) {
+    return await createRelease(
+      tag,
+      config,
+      releaser,
+      owner,
+      repo,
+      discussion_category_name,
+      generate_release_notes,
+      maxRetries,
+      previous_tag_name,
+    );
+  }

+  try {
    let existingRelease: Release = _release!;
    console.log(`Found release ${existingRelease.name} (with id=${existingRelease.id})`);

@@ -733,14 +741,16 @@ export const listReleaseAssets = async (
 /**
 * Finds a release by tag name.
 *
- * Uses the direct getReleaseByTag API for O(1) lookup instead of iterating
- * through all releases. This also avoids GitHub's API pagination limit of
- * 10000 results which would cause failures for repositories with many releases.
+ * Uses the direct getReleaseByTag API for O(1) lookup. Because GitHub does not
+ * expose draft releases through that endpoint, a 404 falls back to a bounded
+ * scan of recent releases and briefly retries in case the listing is not yet
+ * consistent.
 *
 * @param releaser - The GitHub API wrapper for release operations
 * @param owner - The owner of the repository
 * @param repo - The name of the repository
 * @param tag - The tag name to search for
+ * @param listingAttempts - The maximum number of listing attempts after a direct 404
 * @returns The release with the given tag name, or undefined if no release with that tag name is found
 */
 export async function findTagFromReleases(
@@ -748,18 +758,35 @@ export async function findTagFromReleases(
  owner: string,
  repo: string,
  tag: string,
+  listingAttempts: number = 1,
 ): Promise<Release | undefined> {
  try {
    const { data: release } = await releaser.getReleaseByTag({ owner, repo, tag });
    return release;
  } catch (error) {
-    // Release not found (404) or other error - return undefined to allow creation
-    if (error.status === 404) {
-      return undefined;
+    if (error.status !== 404) {
+      throw error;
    }
-    // Re-throw unexpected errors
-    throw error;
  }
+
+  if (listingAttempts <= 0) {
+    return undefined;
+  }
+
+  const attempts = Math.max(1, listingAttempts);
+  for (let attempt = 0; attempt < attempts; attempt += 1) {
+    const recentReleases = await recentReleasesByTag(releaser, owner, repo, tag);
+    const canonicalRelease = pickCanonicalRelease(recentReleases, undefined);
+    if (canonicalRelease) {
+      return canonicalRelease;
+    }
+
+    if (attempt < attempts - 1) {
+      await sleep(CREATED_RELEASE_DISCOVERY_RETRY_DELAY_MS);
+    }
+  }
+
+  return undefined;
 }

 const CREATED_RELEASE_DISCOVERY_RETRY_DELAY_MS = 1000;
@@ -811,33 +838,31 @@ function pickCanonicalRelease(
  })[0];
 }

-async function cleanupDuplicateDraftReleases(
+async function cleanupCreatedDuplicateDraftRelease(
  releaser: Releaser,
  owner: string,
  repo: string,
  tag: string,
  canonicalReleaseId: number,
-  releases: Release[],
+  createdRelease: Release,
 ): Promise<void> {
-  const uniqueReleases = Array.from(
-    new Map(releases.map((release) => [release.id, release])).values(),
-  );
+  if (
+    createdRelease.id === canonicalReleaseId ||
+    !createdRelease.draft ||
+    createdRelease.assets.length > 0
+  ) {
+    return;
+  }

-  for (const duplicate of uniqueReleases) {
-    if (duplicate.id === canonicalReleaseId || !duplicate.draft || duplicate.assets.length > 0) {
-      continue;
-    }
-
-    try {
-      console.log(`🧹 Removing duplicate draft release ${duplicate.id} for tag ${tag}...`);
-      await releaser.deleteRelease({
-        owner,
-        repo,
-        release_id: duplicate.id,
-      });
-    } catch (error) {
-      console.warn(`error deleting duplicate release ${duplicate.id}: ${error}`);
-    }
+  try {
+    console.log(`🧹 Removing duplicate draft release ${createdRelease.id} for tag ${tag}...`);
+    await releaser.deleteRelease({
+      owner,
+      repo,
+      release_id: createdRelease.id,
+    });
+  } catch (error) {
+    console.warn(`error deleting duplicate release ${createdRelease.id}: ${error}`);
  }
 }

@@ -854,7 +879,7 @@ async function canonicalizeCreatedRelease(
  for (let attempt = 1; attempt <= attempts; attempt += 1) {
    let releaseByTag: Release | undefined;
    try {
-      releaseByTag = await findTagFromReleases(releaser, owner, repo, tag);
+      releaseByTag = await findTagFromReleases(releaser, owner, repo, tag, 0);
    } catch (error) {
      console.warn(`error reloading release for tag ${tag}: ${error}`);
    }
@@ -874,10 +899,19 @@ async function canonicalizeCreatedRelease(
        );
      }

-      await cleanupDuplicateDraftReleases(releaser, owner, repo, tag, canonicalRelease.id, [
-        createdRelease,
-        ...recentReleases,
-      ]);
+      const refreshedCreatedRelease = recentReleases.find(
+        (release) => release.id === createdRelease.id,
+      );
+      if (refreshedCreatedRelease) {
+        await cleanupCreatedDuplicateDraftRelease(
+          releaser,
+          owner,
+          repo,
+          tag,
+          canonicalRelease.id,
+          refreshedCreatedRelease,
+        );
+      }
      return canonicalRelease;
    }