fix: Improve download verification and retry logic

cli/cmd/download_package.go

@@ -101,37 +101,54 @@ func (c *DownloadPackageCmd) DownloadBuild(p portal.Portal, build portal.Build,
 	}
 
 	fullFilename := build.BuildPackageFilename(filename)
-	out, err := c.FileWriter.OpenAppend(fullFilename)
-	if err != nil {
-		out, err = c.FileWriter.Create(fullFilename)
-		if err != nil {
-			return fmt.Errorf("failed to create file %s: %w", fullFilename, err)
-		}
-	}
-	defer util.CloseFileIgnoreError(out)
-
-	// get already downloaded file size of fullFilename
-	fileSize := 0
-	fileInfo, err := out.Stat()
-	if err == nil {
-		fileSize = int(fileInfo.Size())
-	}
-
-	err = p.DownloadBuildArtifact("codesphere", download, out, fileSize, c.Opts.Quiet)
-	if err != nil {
-		return fmt.Errorf("failed to download build: %w", err)
-	}
+	for retried := false; ; retried = true {
+		shouldRetry, err := func() (bool, error) {
+			out, err := c.FileWriter.OpenAppend(fullFilename)
+			if err != nil {
+				out, err = c.FileWriter.Create(fullFilename)
+				if err != nil {
+					return false, fmt.Errorf("failed to create file %s: %w", fullFilename, err)
+				}
+			}
+			defer util.CloseFileIgnoreError(out)
+
+			// get already downloaded file size of fullFilename
+			fileSize := 0
+			if fileInfo, statErr := out.Stat(); statErr == nil {
+				fileSize = int(fileInfo.Size())
+			}
+
+			if err = p.DownloadBuildArtifact("codesphere", download, out, fileSize, c.Opts.Quiet); err != nil {
+				return false, fmt.Errorf("failed to download build: %w", err)
+			}
+
+			verifyFile, err := c.FileWriter.Open(fullFilename)
+			if err != nil {
+				return false, err
+			}
+			defer util.CloseFileIgnoreError(verifyFile)
+
+			verifyErr := p.VerifyBuildArtifactDownload(verifyFile, download)
+			if verifyErr == nil {
+				return false, nil
+			}
+
+			// A resumed download can carry stale or corrupt bytes from a previous
+			// interrupted attempt, causing verification to fail even though the
+			// re-download itself succeeded. Delete the partial file so the next
+			// iteration downloads the full artifact from scratch.
+			if !retried && fileSize > 0 {
+				log.Println("Verification failed on resumed download; retrying from scratch...")
+				if removeErr := c.FileWriter.Remove(fullFilename); removeErr == nil {
+					return true, nil
+				}
+			}
 
-	verifyFile, err := c.FileWriter.Open(fullFilename)
-	if err != nil {
-		return err
-	}
-	defer util.CloseFileIgnoreError(verifyFile)
+			return false, fmt.Errorf("failed to verify artifact: %w", verifyErr)
+		}()
 
-	err = p.VerifyBuildArtifactDownload(verifyFile, download)
-	if err != nil {
-		return fmt.Errorf("failed to verify artifact: %w", err)
+		if err != nil || !shouldRetry {
+			return err
+		}
 	}
-
-	return nil
 }

cli/cmd/download_package_test.go

@@ -4,6 +4,7 @@
 package cmd_test
 
 import (
+	"fmt"
 	"os"
 
 	. "github.com/onsi/ginkgo/v2"
@@ -244,4 +245,86 @@ var _ = Describe("DownloadPackages", func() {
 			Expect(err).To(MatchError("failed to find artifact in package: artifact not found: installer-lite.tar.gz"))
 		})
 	})
+
+	Context("Stale partial file causes verify failure", func() {
+		var (
+			expectedBuildToDownload portal.Build
+			fullFilename            string
+			staleFile               *os.File
+			freshFile               *os.File
+		)
+
+		BeforeEach(func() {
+			expectedBuildToDownload = portal.Build{
+				Version: version,
+				Hash:    hash,
+				Artifacts: []portal.Artifact{
+					{Filename: filename},
+				},
+			}
+			fullFilename = version + "-" + hash + "-" + filename
+
+			// Create a temp file with non-zero content to simulate a stale partial download.
+			var err error
+			staleFile, err = os.CreateTemp("", "stale-download-*")
+			Expect(err).NotTo(HaveOccurred())
+			_, err = staleFile.Write([]byte("stale partial content"))
+			Expect(err).NotTo(HaveOccurred())
+			_, err = staleFile.Seek(0, 0)
+			Expect(err).NotTo(HaveOccurred())
+
+			freshFile = os.NewFile(uintptr(0), filename)
+		})
+
+		AfterEach(func() {
+			_ = staleFile.Close() // may already be closed by the code under test
+			Expect(os.Remove(staleFile.Name())).To(Succeed())
+		})
+
+		It("deletes the partial file and retries from scratch when resumed download fails verification", func() {
+			mockFileWriter.EXPECT().OpenAppend(fullFilename).Return(staleFile, nil).Once()
+			mockPortal.EXPECT().DownloadBuildArtifact(portal.CodesphereProduct, expectedBuildToDownload, mock.Anything, mock.AnythingOfType("int"), false).Return(nil).Once()
+			mockFileWriter.EXPECT().Open(fullFilename).Return(staleFile, nil).Once()
+			mockPortal.EXPECT().VerifyBuildArtifactDownload(mock.Anything, expectedBuildToDownload).Return(fmt.Errorf("invalid md5Sum: expected abc, but got xyz")).Once()
+			mockFileWriter.EXPECT().Remove(fullFilename).Return(nil).Once()
+
+			// Retry: fresh download succeeds
+			mockFileWriter.EXPECT().OpenAppend(fullFilename).Return(nil, fmt.Errorf("file not found")).Once()
+			mockFileWriter.EXPECT().Create(fullFilename).Return(freshFile, nil).Once()
+			mockPortal.EXPECT().DownloadBuildArtifact(portal.CodesphereProduct, expectedBuildToDownload, mock.Anything, 0, false).Return(nil).Once()
+			mockFileWriter.EXPECT().Open(fullFilename).Return(freshFile, nil).Once()
+			mockPortal.EXPECT().VerifyBuildArtifactDownload(mock.Anything, expectedBuildToDownload).Return(nil).Once()
+
+			err := c.DownloadBuild(mockPortal, build, filename)
+			Expect(err).NotTo(HaveOccurred())
+		})
+
+		It("returns verify error without retry when file size is zero", func() {
+			emptyFile, createErr := os.CreateTemp("", "empty-download-*")
+			Expect(createErr).NotTo(HaveOccurred())
+			DeferCleanup(func() {
+				_ = emptyFile.Close() // may already be closed by the code under test
+				Expect(os.Remove(emptyFile.Name())).To(Succeed())
+			})
+
+			mockFileWriter.EXPECT().OpenAppend(fullFilename).Return(emptyFile, nil)
+			mockPortal.EXPECT().DownloadBuildArtifact(portal.CodesphereProduct, expectedBuildToDownload, mock.Anything, 0, false).Return(nil)
+			mockFileWriter.EXPECT().Open(fullFilename).Return(emptyFile, nil)
+			mockPortal.EXPECT().VerifyBuildArtifactDownload(mock.Anything, expectedBuildToDownload).Return(fmt.Errorf("invalid md5Sum: expected abc, but got xyz"))
+
+			err := c.DownloadBuild(mockPortal, build, filename)
+			Expect(err).To(MatchError(ContainSubstring("failed to verify artifact")))
+		})
+
+		It("returns verify error when remove of stale file fails", func() {
+			mockFileWriter.EXPECT().OpenAppend(fullFilename).Return(staleFile, nil)
+			mockPortal.EXPECT().DownloadBuildArtifact(portal.CodesphereProduct, expectedBuildToDownload, mock.Anything, mock.AnythingOfType("int"), false).Return(nil)
+			mockFileWriter.EXPECT().Open(fullFilename).Return(staleFile, nil)
+			mockPortal.EXPECT().VerifyBuildArtifactDownload(mock.Anything, expectedBuildToDownload).Return(fmt.Errorf("invalid md5Sum: expected abc, but got xyz"))
+			mockFileWriter.EXPECT().Remove(fullFilename).Return(fmt.Errorf("permission denied"))
+
+			err := c.DownloadBuild(mockPortal, build, filename)
+			Expect(err).To(MatchError(ContainSubstring("failed to verify artifact")))
+		})
+	})
 })