fix: replace existing release assets on Gitea (#816)

Signed-off-by: Rui Chen <rui@chenrui.dev>

__tests__/github.test.ts

@@ -322,6 +322,107 @@ describe('github', () => {
   });
 
   describe('GitHubReleaser', () => {
+    it('uses GitHub standard asset deletion without adding the release ID', async () => {
+      const deleteReleaseAsset = vi.fn().mockResolvedValue(undefined);
+      const request = vi.fn();
+      const releaser = new GitHubReleaser({
+        rest: { repos: { deleteReleaseAsset } },
+        request,
+      } as any);
+
+      await releaser.deleteReleaseAsset({
+        owner: 'owner',
+        repo: 'repo',
+        release_id: 42,
+        asset_id: 99,
+      });
+
+      expect(deleteReleaseAsset).toHaveBeenCalledOnce();
+      expect(deleteReleaseAsset).toHaveBeenCalledWith({
+        owner: 'owner',
+        repo: 'repo',
+        asset_id: 99,
+      });
+      expect(request).not.toHaveBeenCalled();
+    });
+
+    it('falls back to release-scoped asset deletion after a standard 404', async () => {
+      const deleteReleaseAsset = vi.fn().mockRejectedValue({
+        status: 404,
+        message: 'standard route not found',
+      });
+      const request = vi.fn().mockResolvedValue({ status: 204 });
+      const releaser = new GitHubReleaser({
+        rest: { repos: { deleteReleaseAsset } },
+        request,
+      } as any);
+
+      await releaser.deleteReleaseAsset({
+        owner: 'owner',
+        repo: 'repo',
+        release_id: 42,
+        asset_id: 99,
+      });
+
+      expect(request).toHaveBeenCalledOnce();
+      expect(request).toHaveBeenCalledWith(
+        'DELETE /repos/{owner}/{repo}/releases/{release_id}/assets/{asset_id}',
+        {
+          owner: 'owner',
+          repo: 'repo',
+          release_id: 42,
+          asset_id: 99,
+        },
+      );
+    });
+
+    it('does not mask non-404 asset deletion failures', async () => {
+      const deletionError = { status: 403, message: 'forbidden' };
+      const request = vi.fn();
+      const releaser = new GitHubReleaser({
+        rest: {
+          repos: { deleteReleaseAsset: vi.fn().mockRejectedValue(deletionError) },
+        },
+        request,
+      } as any);
+
+      await expect(
+        releaser.deleteReleaseAsset({
+          owner: 'owner',
+          repo: 'repo',
+          release_id: 42,
+          asset_id: 99,
+        }),
+      ).rejects.toBe(deletionError);
+      expect(request).not.toHaveBeenCalled();
+    });
+
+    it('reports both failures when release-scoped deletion also fails', async () => {
+      const releaser = new GitHubReleaser({
+        rest: {
+          repos: {
+            deleteReleaseAsset: vi
+              .fn()
+              .mockRejectedValue({ status: 404, message: 'standard route not found' }),
+          },
+        },
+        request: vi
+          .fn()
+          .mockRejectedValue({ status: 404, message: 'release-scoped route not found' }),
+      } as any);
+
+      await expect(
+        releaser.deleteReleaseAsset({
+          owner: 'owner',
+          repo: 'repo',
+          release_id: 42,
+          asset_id: 99,
+        }),
+      ).rejects.toThrow(
+        'Failed to delete release asset 99. GitHub endpoint: standard route not found; release-scoped endpoint: release-scoped route not found',
+      );
+    });
+
     it('passes previous_tag_name to generateReleaseNotes and strips it from createRelease', async () => {
       const generateReleaseNotes = vi.fn(async () => ({
         data: {
@@ -945,6 +1046,7 @@ describe('github', () => {
         'https://uploads.github.com/repos/owner/repo/releases/1/assets',
         '__tests__/release.txt',
         [],
+        1,
       );
 
       expect(result).toStrictEqual({ id: 123, name: 'release.txt' });
@@ -956,11 +1058,52 @@ describe('github', () => {
       expect(deleteReleaseAsset).toHaveBeenCalledWith({
         owner: 'owner',
         repo: 'repo',
+        release_id: 1,
         asset_id: 99,
       });
       expect(uploadReleaseAsset).toHaveBeenCalledTimes(2);
     });
 
+    it('does not upload until deletion of an existing asset succeeds', async () => {
+      const deletionError = new Error('asset deletion failed');
+      const uploadReleaseAsset = vi.fn();
+      const releaser = createReleaser({
+        deleteReleaseAsset: vi.fn().mockRejectedValue(deletionError),
+        uploadReleaseAsset,
+      });
+
+      await expect(
+        upload(
+          config,
+          releaser,
+          'https://uploads.github.com/repos/owner/repo/releases/1/assets',
+          '__tests__/release.txt',
+          [{ id: 99, name: 'release.txt' }],
+          1,
+        ),
+      ).rejects.toBe(deletionError);
+      expect(uploadReleaseAsset).not.toHaveBeenCalled();
+    });
+
+    it('skips deletion and upload when overwrite_files is false', async () => {
+      const deleteReleaseAsset = vi.fn();
+      const uploadReleaseAsset = vi.fn();
+      const releaser = createReleaser({ deleteReleaseAsset, uploadReleaseAsset });
+
+      await expect(
+        upload(
+          { ...config, input_overwrite_files: false },
+          releaser,
+          'https://uploads.github.com/repos/owner/repo/releases/1/assets',
+          '__tests__/release.txt',
+          [{ id: 99, name: 'release.txt' }],
+          1,
+        ),
+      ).resolves.toBeNull();
+      expect(deleteReleaseAsset).not.toHaveBeenCalled();
+      expect(uploadReleaseAsset).not.toHaveBeenCalled();
+    });
+
     it('surfaces an actionable immutable-release error for prerelease uploads', async () => {
       const tempDir = mkdtempSync(join(tmpdir(), 'gh-release-immutable-'));
       const assetPath = join(tempDir, 'draft-false.txt');
@@ -1000,6 +1143,7 @@ describe('github', () => {
           'https://uploads.github.com/repos/owner/repo/releases/1/assets',
           assetPath,
           [],
+          1,
         ),
       ).rejects.toThrow(
         'Cannot upload asset draft-false.txt to an immutable release. GitHub only allows asset uploads before a release is published, but draft prereleases publish with the release.published event instead of release.prereleased.',
@@ -1054,6 +1198,7 @@ describe('github', () => {
           'https://uploads.github.com/repos/owner/repo/releases/1/assets',
           dotfilePath,
           [],
+          1,
         );
 
         expect(result).toStrictEqual({ id: 123, name: 'default.config', label: '.config' });
@@ -1065,6 +1210,7 @@ describe('github', () => {
         expect(deleteReleaseAsset).toHaveBeenCalledWith({
           owner: 'owner',
           repo: 'repo',
+          release_id: 1,
           asset_id: 99,
         });
         expect(updateReleaseAsset).toHaveBeenCalledWith({
@@ -1512,6 +1658,7 @@ describe('github', () => {
           'https://uploads.example.test/assets',
           dotfilePath,
           [],
+          1,
         );
 
         expect(updateReleaseAssetSpy).toHaveBeenCalledWith({
@@ -1583,6 +1730,7 @@ describe('github', () => {
           'https://uploads.github.com/repos/owner/repo/releases/1/assets',
           dotfilePath,
           [],
+          1,
         );
 
         expect(updateReleaseAssetSpy).toHaveBeenNthCalledWith(1, {
@@ -1653,6 +1801,7 @@ describe('github', () => {
           'https://uploads.github.com/repos/owner/repo/releases/1/assets',
           dotfilePath,
           [],
+          1,
         );
 
         expect(listReleaseAssetsSpy).toHaveBeenCalledWith({
@@ -1712,6 +1861,7 @@ describe('github', () => {
           'https://uploads.github.com/repos/owner/repo/releases/1/assets',
           dotfilePath,
           [],
+          1,
         );
 
         expect(listReleaseAssetsSpy).toHaveBeenCalledWith({
@@ -1772,6 +1922,7 @@ describe('github', () => {
           'https://uploads.github.com/repos/owner/repo/releases/1/assets',
           dotfilePath,
           [],
+          1,
         );
 
         await vi.waitFor(() => expect(listReleaseAssetsSpy).toHaveBeenCalledTimes(1));
@@ -1827,18 +1978,26 @@ describe('github', () => {
       };
 
       try {
-        await upload(config, releaser, 'https://uploads.example.test/assets', dotfilePath, [
-          {
-            id: 1,
-            name: 'default.config',
-            label: '.config',
-          },
-        ]);
+        await upload(
+          config,
+          releaser,
+          'https://uploads.example.test/assets',
+          dotfilePath,
+          [
+            {
+              id: 1,
+              name: 'default.config',
+              label: '.config',
+            },
+          ],
+          1,
+        );
 
         expect(deleteReleaseAssetSpy).toHaveBeenCalledWith({
           asset_id: 1,
           owner: 'owner',
           repo: 'repo',
+          release_id: 1,
         });
       } finally {
         rmSync(tempDir, { recursive: true, force: true });

__tests__/run.test.ts

@@ -211,6 +211,7 @@ describe('run', () => {
       'https://uploads.example.test/releases/41/assets',
       'fixture/dist/action.zip',
       initialRelease.assets,
+      initialRelease.id,
     );
   });
 

__tests__/upload-stream.test.ts

@@ -205,7 +205,7 @@ describe('release asset upload transport', () => {
       openFile.mockClear();
 
       try {
-        await expect(upload(config, releaser, uploadUrl, fixture.path, [])).resolves.toEqual({
+        await expect(upload(config, releaser, uploadUrl, fixture.path, [], 1)).resolves.toEqual({
           id: 123,
           name: fixture.name,
         });
@@ -243,7 +243,7 @@ describe('release asset upload transport', () => {
     openFile.mockClear();
 
     try {
-      await expect(upload(config, releaser, uploadUrl, fixture.path, [])).rejects.toThrow(
+      await expect(upload(config, releaser, uploadUrl, fixture.path, [], 1)).rejects.toThrow(
         'Validation Failed',
       );
       await expectLastFileHandleClosed();
@@ -275,7 +275,7 @@ describe('release asset upload transport', () => {
     });
 
     try {
-      await expect(upload(config, releaser, uploadUrl, fixture.path, [])).resolves.toEqual({
+      await expect(upload(config, releaser, uploadUrl, fixture.path, [], 1)).resolves.toEqual({
         id: 123,
         name: fixture.name,
       });
@@ -297,11 +297,13 @@ describe('release asset upload transport', () => {
     );
     const releaser = new GitHubReleaser(getOctokit(config.github_token));
     vi.spyOn(releaser, 'listReleaseAssets').mockResolvedValue([{ id: 9, name: fixture.name }]);
-    vi.spyOn(releaser, 'deleteReleaseAsset').mockResolvedValue(undefined);
+    const deleteReleaseAsset = vi
+      .spyOn(releaser, 'deleteReleaseAsset')
+      .mockResolvedValue(undefined);
     openFile.mockClear();
 
     try {
-      await expect(upload(config, releaser, uploadUrl, fixture.path, [])).resolves.toEqual({
+      await expect(upload(config, releaser, uploadUrl, fixture.path, [], 1)).resolves.toEqual({
         id: 123,
         name: fixture.name,
       });
@@ -309,6 +311,12 @@ describe('release asset upload transport', () => {
       expect(receipts.map(({ size }) => size)).toEqual([fixture.size, fixture.size]);
       expect(receipts.map(({ digest }) => digest)).toEqual([fixture.digest, fixture.digest]);
       expect(openFile).toHaveBeenCalledTimes(2);
+      expect(deleteReleaseAsset).toHaveBeenCalledWith({
+        owner: 'owner',
+        repo: 'repo',
+        release_id: 1,
+        asset_id: 9,
+      });
       for (const result of openFile.mock.results) {
         const fileHandle = await result.value;
         await expect(fileHandle.stat()).rejects.toMatchObject({ code: 'EBADF' });
@@ -317,4 +325,70 @@ describe('release asset upload transport', () => {
       await closeServer(server);
     }
   });
+
+  it('falls back to the release-scoped delete route through the real Octokit request path', async () => {
+    const requests: Array<{
+      method: string | undefined;
+      pathname: string;
+      authorization: string | undefined;
+    }> = [];
+    const server = createServer((request, response) => {
+      const url = new URL(request.url || '/', 'http://127.0.0.1');
+      requests.push({
+        method: request.method,
+        pathname: url.pathname,
+        authorization: request.headers.authorization,
+      });
+
+      if (url.pathname === '/repos/owner/repo/releases/assets/9') {
+        response.writeHead(404, { 'content-type': 'application/json' });
+        response.end(JSON.stringify({ message: 'page not found' }));
+        return;
+      }
+      if (url.pathname === '/repos/owner/repo/releases/1/assets/9') {
+        response.writeHead(204);
+        response.end();
+        return;
+      }
+
+      response.writeHead(500, { 'content-type': 'application/json' });
+      response.end(JSON.stringify({ message: 'unexpected route' }));
+    });
+
+    await new Promise<void>((resolve, reject) => {
+      server.once('error', reject);
+      server.listen(0, '127.0.0.1', resolve);
+    });
+    const address = server.address() as AddressInfo;
+    const releaser = new GitHubReleaser(
+      getOctokit(config.github_token, {
+        baseUrl: `http://127.0.0.1:${address.port}`,
+      }),
+    );
+
+    try {
+      await expect(
+        releaser.deleteReleaseAsset({
+          owner: 'owner',
+          repo: 'repo',
+          release_id: 1,
+          asset_id: 9,
+        }),
+      ).resolves.toBeUndefined();
+      expect(requests).toEqual([
+        {
+          method: 'DELETE',
+          pathname: '/repos/owner/repo/releases/assets/9',
+          authorization: 'token not-a-real-token',
+        },
+        {
+          method: 'DELETE',
+          pathname: '/repos/owner/repo/releases/1/assets/9',
+          authorization: 'token not-a-real-token',
+        },
+      ]);
+    } finally {
+      await closeServer(server);
+    }
+  });
 });

src/github.ts

@@ -96,7 +96,12 @@ export interface Releaser {
     release_id: number;
   }): Promise<Array<{ id: number; name: string; label?: string | null; [key: string]: any }>>;
 
-  deleteReleaseAsset(params: { owner: string; repo: string; asset_id: number }): Promise<void>;
+  deleteReleaseAsset(params: {
+    owner: string;
+    repo: string;
+    release_id: number;
+    asset_id: number;
+  }): Promise<void>;
 
   deleteRelease(params: { owner: string; repo: string; release_id: number }): Promise<void>;
 
@@ -229,9 +234,34 @@ export class GitHubReleaser implements Releaser {
   async deleteReleaseAsset(params: {
     owner: string;
     repo: string;
+    release_id: number;
     asset_id: number;
   }): Promise<void> {
-    await this.github.rest.repos.deleteReleaseAsset(params);
+    const { release_id, ...githubParams } = params;
+    try {
+      await this.github.rest.repos.deleteReleaseAsset(githubParams);
+    } catch (error: unknown) {
+      const status =
+        (error as { status?: number; response?: { status?: number } })?.status ??
+        (error as { response?: { status?: number } })?.response?.status;
+      if (status !== 404) {
+        throw error;
+      }
+
+      try {
+        await this.github.request(
+          'DELETE /repos/{owner}/{repo}/releases/{release_id}/assets/{asset_id}',
+          params,
+        );
+      } catch (fallbackError: unknown) {
+        throw new AggregateError(
+          [error, fallbackError],
+          `Failed to delete release asset ${params.asset_id}. GitHub endpoint: ${errorMessage(
+            error,
+          )}; release-scoped endpoint: ${errorMessage(fallbackError)}`,
+        );
+      }
+    }
   }
 
   async deleteRelease(params: { owner: string; repo: string; release_id: number }): Promise<void> {
@@ -321,11 +351,10 @@ export const upload = async (
   url: string,
   path: string,
   currentAssets: Array<{ id: number; name: string; label?: string | null }>,
+  releaseId: number,
 ): Promise<any> => {
   const [owner, repo] = config.github_repository.split('/');
   const { name, mime, size } = asset(path);
-  const releaseIdMatch = url.match(/\/releases\/(\d+)\/assets/);
-  const releaseId = releaseIdMatch ? Number(releaseIdMatch[1]) : undefined;
   const currentAsset = currentAssets.find(
     // GitHub can rewrite uploaded asset names, so compare against both the raw name
     // GitHub returns and the restored label we set when available.
@@ -341,6 +370,7 @@ export const upload = async (
         asset_id: currentAsset.id || 1,
         owner,
         repo,
+        release_id: releaseId,
       });
     }
   }
@@ -499,6 +529,7 @@ export const upload = async (
         await releaser.deleteReleaseAsset({
           owner,
           repo,
+          release_id: releaseId,
           asset_id: latestAsset.id,
         });
         return await handleUploadedAsset(await uploadAsset());

src/run.ts

@@ -52,7 +52,14 @@ export async function run(): Promise<void> {
       const currentAssets = rel.assets;
 
       const uploadFile = async (path: string) => {
-        const json = await upload(config, releaser, uploadUrl(rel.upload_url), path, currentAssets);
+        const json = await upload(
+          config,
+          releaser,
+          uploadUrl(rel.upload_url),
+          path,
+          currentAssets,
+          rel.id,
+        );
         return json ? (json.id as number) : undefined;
       };