From 08ba777f2b42deb1d47f30b454e687676ed66f62 Mon Sep 17 00:00:00 2001 From: bircni Date: Sun, 18 Oct 2026 17:19:54 +0200 Subject: [PATCH] fix(release): upload before deleting on --clobber; show repo view to readers without code access Signed-off-by: bircni --- internal/app/release_assets.go | 15 ++++++++++----- internal/app/release_assets_test.go | 2 +- internal/services/repositories.go | 3 ++- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/internal/app/release_assets.go b/internal/app/release_assets.go index cdfc2a25..0ea4ad32 100644 --- a/internal/app/release_assets.go +++ b/internal/app/release_assets.go @@ -129,24 +129,29 @@ $ tea release upload v1.0.0 dist/tea-linux --clobber`, if err != nil { return err } - replaced := map[string]services.ReleaseAsset{} + replaced := map[string][]services.ReleaseAsset{} for _, file := range files { for _, asset := range release.Assets { if asset.Name == filename(file) { if !clobber { return clierr.Wrap(clierr.Usage, fmt.Errorf("asset %s already exists in release %s; pass --clobber to replace it", asset.Name, release.Tag)) } - replaced[asset.Name] = asset + replaced[asset.Name] = append(replaced[asset.Name], asset) } } } return batchDone(a, cmd, files, func(file *os.File) string { return "upload asset " + filename(file) }, func(file *os.File) (services.ReleaseAsset, error) { - if old, ok := replaced[filename(file)]; ok { + // Upload first: a failed upload must not leave the release without the old asset. + uploaded, err := service.UploadAsset(cmd.Context(), release.ID, filename(file), file) + if err != nil { + return services.ReleaseAsset{}, err + } + for _, old := range replaced[filename(file)] { if err := service.DeleteAsset(cmd.Context(), release.ID, old.ID); err != nil { - return services.ReleaseAsset{}, fmt.Errorf("replace asset %s: %w", old.Name, err) + return uploaded, fmt.Errorf("uploaded %s, but could not delete the replaced asset %d: %w", old.Name, old.ID, err) } } - return service.UploadAsset(cmd.Context(), release.ID, filename(file), file) + return uploaded, nil }, func(asset services.ReleaseAsset) string { return fmt.Sprintf("Uploaded %s (%s) to release %s", asset.Name, terminal.Bytes(asset.Size), release.Tag) }) diff --git a/internal/app/release_assets_test.go b/internal/app/release_assets_test.go index d03f0d1b..1a54d10d 100644 --- a/internal/app/release_assets_test.go +++ b/internal/app/release_assets_test.go @@ -66,7 +66,7 @@ func TestReleaseUploadRefusesDuplicateNamesWithoutClobber(t *testing.T) { require.Equal(t, []string{"GET /api/v1/repos/owner/repo/releases/tags/v1"}, requests) requests = nil require.NoError(t, Execute(context.Background(), deps, []string{"release", "upload", "v1", file, "--clobber", "-R", "owner/repo"})) - require.Equal(t, []string{"GET /api/v1/repos/owner/repo/releases/tags/v1", "DELETE /api/v1/repos/owner/repo/releases/1/assets/7", "POST /api/v1/repos/owner/repo/releases/1/assets"}, requests) + require.Equal(t, []string{"GET /api/v1/repos/owner/repo/releases/tags/v1", "POST /api/v1/repos/owner/repo/releases/1/assets", "DELETE /api/v1/repos/owner/repo/releases/1/assets/7"}, requests) require.Equal(t, "✓ Uploaded hello.txt (5 B) to release v1\n", diagnostics.String()) } diff --git a/internal/services/repositories.go b/internal/services/repositories.go index 0f3bf3b4..90e4a1d3 100644 --- a/internal/services/repositories.go +++ b/internal/services/repositories.go @@ -109,7 +109,8 @@ func (s Repositories) Readme(ctx context.Context, repo, ref string) (name, conte return c.Repositories.ListContents(ctx, owner, name, ref, "") }) if err != nil { - if IsNotFound(err) { + // A reader without code access sees the repository but not its files. + if IsNotFound(err) || IsForbidden(err) { return "", "", nil } return "", "", err