fix(cli): classify errored provider purchase results as failures

cmd/multi_service.go

@@ -596,11 +596,22 @@ func probeAuditLog(isDryRun bool, auditLogPath string) error {
 	return nil
 }
 
+// newServiceClient is the service-client factory used by purchaseSingleRec.
+// It is a seam so tests can drive the real purchase path with a fake provider.
+var newServiceClient = createServiceClient
+
+// purchaseSucceeded reports whether a purchase result is a success: the
+// provider set Success AND reported no error. A provider may return
+// Success=true with a non-nil Error; that is a failure (issue #1900).
+func purchaseSucceeded(result common.PurchaseResult) bool {
+	return result.Success && result.Error == nil
+}
+
 // purchaseAuditStatus derives the audit status for a completed (non-dry-run)
 // purchase attempt. Callers handle the dry-run ("skipped") case separately,
 // since that never reaches a PurchaseResult from an actual API call.
 func purchaseAuditStatus(result common.PurchaseResult) string {
-	if result.Success {
+	if purchaseSucceeded(result) {
 		return "success"
 	}
 	return "error"
@@ -627,15 +638,15 @@ func purchaseSingleRec(ctx context.Context, awsCfg aws.Config, rec common.Recomm
 	}
 	regionalCfg := awsCfg.Copy()
 	regionalCfg.Region = clientRegion
-	serviceClient := createServiceClient(rec.Service, regionalCfg)
+	serviceClient := newServiceClient(rec.Service, regionalCfg)
 	if serviceClient == nil {
 		AppLogger.Printf("    ⚠️  No service client for %s\n", rec.Service)
 		return common.PurchaseResult{Success: false}, "error"
 	}
 
 	result := executePurchase(ctx, rec, label, index, serviceClient, cfg)
 	status := purchaseAuditStatus(result)
-	if result.Success {
+	if purchaseSucceeded(result) {
 		AppLogger.Printf("    ✅ %s\n", result.CommitmentID)
 	} else {
 		AppLogger.Printf("    ❌ %v\n", result.Error)
@@ -1048,7 +1059,7 @@ func runOnePurchase(ctx context.Context, rec common.Recommendation, label, regio
 }
 
 func logPurchaseOutcome(result common.PurchaseResult) {
-	if result.Success {
+	if purchaseSucceeded(result) {
 		AppLogger.Printf("    ✅ Success: %s\n", result.CommitmentID)
 		return
 	}

cmd/purchase_result_classification_test.go

@@ -0,0 +1,88 @@
+package main
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
+	"github.com/LeanerCloud/cloud-commitments-go/pkg/provider"
+	"github.com/aws/aws-sdk-go-v2/aws"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+)
+
+// purchaseClassificationCases covers how a provider's PurchaseCommitment
+// return is classified (issue #1900): a result is a success only when
+// Success is true AND Error is nil.
+func purchaseClassificationCases() []struct {
+	name       string
+	result     common.PurchaseResult
+	err        error
+	wantStatus string
+	wantMarker string
+} {
+	providerErr := errors.New("provider reported partial failure")
+	return []struct {
+		name       string
+		result     common.PurchaseResult
+		err        error
+		wantStatus string
+		wantMarker string
+	}{
+		{"success", common.PurchaseResult{Success: true, CommitmentID: "ri-ok"}, nil, "success", "✅"},
+		{"success flag with provider error", common.PurchaseResult{Success: true, CommitmentID: "ri-err", Error: providerErr}, nil, "error", "❌"},
+		{"success false", common.PurchaseResult{Success: false, Error: providerErr}, nil, "error", "❌"},
+		{"separate go error", common.PurchaseResult{Success: true, CommitmentID: "ri-goerr"}, errors.New("api call failed"), "error", "❌"},
+	}
+}
+
+func TestPurchaseSingleRec_ClassifiesErroredProviderResultAsFailure(t *testing.T) {
+	t.Setenv("DISABLE_PURCHASE_DELAY", "true")
+	for _, tc := range purchaseClassificationCases() {
+		t.Run(tc.name, func(t *testing.T) {
+			client := &MockServiceClient{}
+			client.On("PurchaseCommitment", mock.Anything, mock.Anything, mock.Anything).Return(tc.result, tc.err)
+			oldFactory := newServiceClient
+			newServiceClient = func(common.ServiceType, aws.Config) provider.ServiceClient { return client }
+			t.Cleanup(func() { newServiceClient = oldFactory })
+
+			cfg := Config{CallerAccountID: defaultTestAccount}
+			var status string
+			out := captureAppOutput(t, func() {
+				_, status = purchaseSingleRec(context.Background(), aws.Config{}, validEC2Rec(), 1, false, cfg)
+			})
+
+			client.AssertNumberOfCalls(t, "PurchaseCommitment", 1)
+			assert.Equal(t, tc.wantStatus, status)
+			assert.Contains(t, out, tc.wantMarker)
+			if tc.wantStatus == "error" {
+				assert.NotContains(t, out, "✅")
+			}
+		})
+	}
+}
+
+func TestRunOnePurchase_ClassifiesErroredProviderResultAsFailure(t *testing.T) {
+	for _, tc := range purchaseClassificationCases() {
+		t.Run(tc.name, func(t *testing.T) {
+			client := &MockServiceClient{}
+			client.On("PurchaseCommitment", mock.Anything, mock.Anything, mock.Anything).Return(tc.result, tc.err)
+			rec := validEC2Rec()
+			cfg := Config{CallerAccountID: defaultTestAccount}
+
+			var status string
+			out := captureAppOutput(t, func() {
+				var result common.PurchaseResult
+				result, status, _ = runOnePurchase(context.Background(), rec, rec.Region, rec.Region, 1, false, client, cfg)
+				logPurchaseOutcome(result)
+			})
+
+			assert.Equal(t, tc.wantStatus, status)
+			assert.Contains(t, out, tc.wantMarker)
+			if tc.wantStatus == "error" {
+				assert.NotContains(t, out, "✅")
+			}
+		})
+	}
+}