Fix macOS package selection for universal apps

pkg/appstore/appstore_download_package.go

@@ -12,13 +12,21 @@ func downloadPackagePlatform(platform Platform, item downloadItemResult) (Platfo
 
 	softwarePlatform := downloadMetadataString(item.Metadata, "software-platform")
 	productType := downloadMetadataString(item.Metadata, "product-type")
-	mobileMetadata := strings.EqualFold(softwarePlatform, "ios") || strings.EqualFold(productType, "ios-app")
-	macMetadata := strings.EqualFold(softwarePlatform, "macos") || strings.EqualFold(productType, "mac-os-app")
+	// Universal Mac apps can retain product-type=ios-app. Prefer the
+	// package's explicit software platform over that catalog classification.
+	mobileMetadata := strings.EqualFold(softwarePlatform, "ios")
+	macMetadata := strings.EqualFold(softwarePlatform, "macos")
+
+	if softwarePlatform == "" {
+		mobileMetadata = strings.EqualFold(productType, "ios-app")
+		macMetadata = strings.EqualFold(productType, "mac-os-app")
+	}
+
 	mobileSinf := hasMobileSinf(item.Sinfs)
 	mobileSinfEvidence := hasMobileSinfEvidence(item.Sinfs)
 	macDPInfo := hasMacDPInfo(item.Sinfs)
 
-	if (mobileMetadata && macMetadata) || (mobileMetadata && macDPInfo) || (macMetadata && mobileSinfEvidence) || (mobileSinfEvidence && macDPInfo) {
+	if (mobileMetadata && macDPInfo) || (macMetadata && mobileSinfEvidence) || (mobileSinfEvidence && macDPInfo) {
 		return "", errors.New("download response contains conflicting package metadata")
 	}
 

pkg/appstore/appstore_download_package_test.go

@@ -20,6 +20,18 @@ var _ = Describe("Download package platform", func() {
 		Expect(platform).To(Equal(PlatformMacOS))
 	})
 
+	It("recognizes universal Mac packages with an iOS product type", func() {
+		platform, err := downloadPackagePlatform(PlatformMacOS, downloadItemResult{
+			Metadata: map[string]interface{}{
+				"software-platform": "macos",
+				"product-type":      "ios-app",
+			},
+			Sinfs: []Sinf{{DPInfo: make([]byte, 88)}},
+		})
+		Expect(err).ToNot(HaveOccurred())
+		Expect(platform).To(Equal(PlatformMacOS))
+	})
+
 	It("accepts native authorization data that also contains sinf data", func() {
 		item := downloadItemResult{
 			Metadata: map[string]interface{}{

pkg/appstore/appstore_download_product.go

@@ -24,6 +24,17 @@ type downloadProductEndpoint struct {
 }
 
 func (t *appstore) sendDownloadProduct(acc Account, app App, guid, externalVersionID string, platform Platform) (http.Result[downloadResult], Platform, error) {
+	// An unpinned request can return the iOS build of a universal app.
+	// Select the Mac offer before accepting any download response.
+	if externalVersionID == "" && platform == PlatformMacOS {
+		var err error
+
+		externalVersionID, err = t.lookupLatestMacOSExternalVersionID(acc, app)
+		if err != nil {
+			return http.Result[downloadResult]{}, platform, fmt.Errorf("failed to resolve latest macOS version for download: %w", err)
+		}
+	}
+
 	volumeStore := t.volumeStoreEndpoint(acc)
 
 	res, err := t.downloadClient.Send(t.downloadProductRequest(volumeStore, acc, app, guid, externalVersionID))
@@ -35,24 +46,6 @@ func (t *appstore) sendDownloadProduct(acc Account, app App, guid, externalVersi
 		return res, platform, nil
 	}
 
-	if externalVersionID == "" && platform == PlatformMacOS {
-		externalVersionID, err = t.lookupLatestMacOSExternalVersionID(acc, app)
-		if err != nil {
-			return res, platform, fmt.Errorf("failed to resolve latest macOS version for download: %w", err)
-		}
-
-		// Pin the Mac offer on volumeStore itself. The redownload endpoint
-		// can still return an empty HTTP 500 for the same pinned Mac version.
-		res, err = t.downloadClient.Send(t.downloadProductRequest(volumeStore, acc, app, guid, externalVersionID))
-		if err != nil {
-			return res, platform, fmt.Errorf("failed to send pinned macOS download request: %w", err)
-		}
-
-		if !isEmptyDownloadProductResponse(res) && !isUnavailableDownloadProductResponse(res) {
-			return res, platform, nil
-		}
-	}
-
 	// Try redownload when volumeStore returns no items, either silently or
 	// with the message-only "No Longer Available" response.
 	bag, err := t.fetchURLBag(guid)

pkg/appstore/appstore_download_product_test.go

@@ -197,7 +197,7 @@ var _ = Describe("AppStore (Download Product)", func() {
 			}
 			mockDownloadClient.EXPECT().Send(gomock.Any()).Return(expected, nil)
 
-			actual, resolvedPlatform, err := store.sendDownloadProduct(account, app, testGUID, "", platform)
+			actual, resolvedPlatform, err := store.sendDownloadProduct(account, app, testGUID, testVersionID, platform)
 			Expect(err).ToNot(HaveOccurred())
 			Expect(actual).To(Equal(expected))
 			Expect(resolvedPlatform).To(Equal(platform))

pkg/appstore/appstore_download_test.go

@@ -858,7 +858,7 @@ var _ = Describe("AppStore (Download)", func() {
 							Metadata: map[string]interface{}{
 								"bundleShortVersionString": "1.2.3",
 								"software-platform":        "macos",
-								"product-type":             "mac-os-app",
+								"product-type":             "ios-app",
 							},
 						}},
 					},
@@ -889,10 +889,11 @@ var _ = Describe("AppStore (Download)", func() {
 				},
 			}
 			out, err := store.Download(DownloadInput{
-				Context:    context.Background(),
-				App:        App{ID: 42, BundleID: "com.example.mac"},
-				OutputPath: requestedPath,
-				Platform:   PlatformMacOS,
+				Context:           context.Background(),
+				App:               App{ID: 42, BundleID: "com.example.mac"},
+				OutputPath:        requestedPath,
+				Platform:          PlatformMacOS,
+				ExternalVersionID: "123456789",
 			})
 
 			Expect(err).ToNot(HaveOccurred())
@@ -965,11 +966,12 @@ var _ = Describe("AppStore (Download)", func() {
 			}
 
 			out, err := store.Download(DownloadInput{
-				Context:    context.Background(),
-				Account:    Account{Email: "test@example.com"},
-				App:        App{ID: 640199958, BundleID: "developer.apple.wwdc-Release"},
-				OutputPath: tempDir,
-				Platform:   PlatformMacOS,
+				Context:           context.Background(),
+				Account:           Account{Email: "test@example.com"},
+				App:               App{ID: 640199958, BundleID: "developer.apple.wwdc-Release"},
+				OutputPath:        tempDir,
+				Platform:          PlatformMacOS,
+				ExternalVersionID: "123456789",
 			})
 
 			Expect(err).ToNot(HaveOccurred())

pkg/appstore/appstore_macos_version_lookup_test.go

@@ -38,20 +38,14 @@ var _ = Describe("Mac purchase version selection", func() {
 		Entry("conflicting offers", `[`+karingMacConfiguration+`,{"purchaseConfiguration":{"bundleId":"com.nebula.karing","appPlatforms":["mac"],"buyParams":"salableAdamId=6472431552&appExtVrsId=123"}}]`),
 	)
 
-	DescribeTable("retries volumeStore with the Mac version when no package is returned", func(unavailable bool, downloadErr error) {
+	DescribeTable("pins the Mac offer before requesting a package", func(downloadErr error) {
 		ctrl := gomock.NewController(GinkgoT())
 		defer ctrl.Finish()
 		downloads := http.NewMockClient[downloadResult](ctrl)
-		bags := http.NewMockClient[bagResult](ctrl)
 		pages := http.NewMockClient[[]byte](ctrl)
-		store := &appstore{downloadClient: downloads, bagClient: bags, storefrontClient: pages}
+		store := &appstore{downloadClient: downloads, storefrontClient: pages}
 		expected := http.Result[downloadResult]{StatusCode: gohttp.StatusOK, Data: downloadResult{Items: []downloadItemResult{{URL: "https://example.test/mac.pkg"}}}}
-		initial := http.Result[downloadResult]{StatusCode: gohttp.StatusOK}
-		if unavailable {
-			initial.Data.CustomerMessage = "No Longer Available"
-		}
 		gomock.InOrder(
-			downloads.EXPECT().Send(gomock.Any()).Return(initial, nil),
 			pages.EXPECT().Send(gomock.Any()).Do(func(req http.Request) {
 				Expect(req.URL).To(Equal("https://apps.apple.com/de/app/id6472431552?platform=mac"))
 			}).Return(http.Result[[]byte]{StatusCode: gohttp.StatusOK, Data: macVersionPage(karingMacConfiguration)}, nil),
@@ -69,18 +63,47 @@ var _ = Describe("Mac purchase version selection", func() {
 			Expect(errors.Is(err, downloadErr)).To(BeTrue())
 		}
 	},
-		Entry("empty primary response", false, nil),
-		Entry("unavailable primary response", true, nil),
-		Entry("propagates pinned download failure", false, &http.UnexpectedResponseError{StatusCode: gohttp.StatusInternalServerError}),
+		Entry("successful primary response", nil),
+		Entry("propagates pinned download failure", &http.UnexpectedResponseError{StatusCode: gohttp.StatusInternalServerError}),
+	)
+
+	DescribeTable("keeps the Mac version pinned through redownload fallback", func(message string) {
+		ctrl := gomock.NewController(GinkgoT())
+		defer ctrl.Finish()
+		downloads := http.NewMockClient[downloadResult](ctrl)
+		pages := http.NewMockClient[[]byte](ctrl)
+		bags := http.NewMockClient[bagResult](ctrl)
+		store := &appstore{downloadClient: downloads, storefrontClient: pages, bagClient: bags}
+		bag := validBagResult()
+		bag.URLBag.RedownloadEndpoint = testRedownloadEndpoint
+		expected := http.Result[downloadResult]{StatusCode: gohttp.StatusOK, Data: downloadResult{Items: []downloadItemResult{{URL: "https://example.test/mac.pkg"}}}}
+		gomock.InOrder(
+			pages.EXPECT().Send(gomock.Any()).Return(http.Result[[]byte]{StatusCode: gohttp.StatusOK, Data: macVersionPage(karingMacConfiguration)}, nil),
+			downloads.EXPECT().Send(gomock.Any()).Do(func(req http.Request) {
+				Expect(req.Payload.(*http.XMLPayload).Content).To(HaveKeyWithValue("externalVersionId", "876660716"))
+			}).Return(http.Result[downloadResult]{StatusCode: gohttp.StatusOK, Data: downloadResult{CustomerMessage: message}}, nil),
+			bags.EXPECT().Send(gomock.Any()).Return(http.Result[bagResult]{StatusCode: gohttp.StatusOK, Data: bag}, nil),
+			downloads.EXPECT().Send(gomock.Any()).Do(func(req http.Request) {
+				Expect(req.URL).To(Equal(testRedownloadEndpoint + "?guid=001122334455"))
+				Expect(req.Payload.(*http.XMLPayload).Content).To(HaveKeyWithValue("appExtVrsId", "876660716"))
+			}).Return(expected, nil),
+		)
+		output, platform, err := store.sendDownloadProduct(Account{StoreFront: "143443-2,34"}, app, "001122334455", "", PlatformMacOS)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(output).To(Equal(expected))
+		Expect(platform).To(Equal(PlatformMacOS))
+	},
+		Entry("empty response", ""),
+		Entry("unavailable response", "No Longer Available"),
 	)
 
 	DescribeTable("propagates lookup failures", func(status int, body []byte, requestErr error) {
 		ctrl := gomock.NewController(GinkgoT())
 		defer ctrl.Finish()
 		pages := http.NewMockClient[[]byte](ctrl)
-		store := &appstore{storefrontClient: pages}
+		store := &appstore{storefrontClient: pages, downloadClient: http.NewMockClient[downloadResult](ctrl)}
 		pages.EXPECT().Send(gomock.Any()).Return(http.Result[[]byte]{StatusCode: status, Data: body}, requestErr)
-		_, err := store.lookupLatestMacOSExternalVersionID(Account{StoreFront: "143443-2,34"}, app)
+		_, _, err := store.sendDownloadProduct(Account{StoreFront: "143443-2,34"}, app, "001122334455", "", PlatformMacOS)
 		Expect(err).To(HaveOccurred())
 	},
 		Entry("HTTP failure", 500, nil, nil),