fix(reconcile): honor a product's stated behavior on create and match feature-name case on apply

billing/product/service.go

@@ -64,8 +64,12 @@ func (s *Service) Create(ctx context.Context, product Product) (Product, error)
 		product.ID = uuid.New().String()
 		product.ProviderID = product.ID
 	}
+	// Capture whether the caller stated a behavior before SetDefaults fills the
+	// "basic" default, so an explicit behavior is honored and only an omitted one
+	// on a credit product falls back to "credits".
+	statedBehavior := product.Behavior
 	defaults.SetDefaults(&product)
-	if product.Config.CreditAmount > 0 {
+	if statedBehavior == "" && product.Config.CreditAmount > 0 {
 		product.Behavior = CreditBehavior
 	}
 	product.Name = strings.ToLower(product.Name)

billing/product/service_test.go

@@ -85,6 +85,109 @@ func TestService_Create(t *testing.T) {
 				return product.NewService(stripeClient, mockProductRepo, mockPriceRepo, mockFeatureRepo)
 			},
 		},
+		{
+			name: "honors an explicit behavior on a credit product instead of forcing credits",
+			args: args{
+				product: product.Product{
+					ID:          "creditprod",
+					Name:        "creditprod",
+					Description: "credit product",
+					Behavior:    product.PerSeatBehavior,
+					Config:      product.BehaviorConfig{CreditAmount: 5},
+				},
+			},
+			want: product.Product{
+				ID:          "creditprod",
+				Name:        "creditprod",
+				Description: "credit product",
+				Behavior:    product.PerSeatBehavior,
+				Config:      product.BehaviorConfig{CreditAmount: 5},
+			},
+			wantErr: false,
+			setup: func() *product.Service {
+				stripeClient, mockStripeBackend, mockProductRepo, mockPriceRepo, mockFeatureRepo := mockService(t)
+				mockProductRepo.EXPECT().Create(ctx, product.Product{
+					ID:          "creditprod",
+					Name:        "creditprod",
+					Description: "credit product",
+					Behavior:    product.PerSeatBehavior,
+					Config:      product.BehaviorConfig{CreditAmount: 5},
+				}).Return(product.Product{
+					ID:          "creditprod",
+					Name:        "creditprod",
+					Description: "credit product",
+					Behavior:    product.PerSeatBehavior,
+					Config:      product.BehaviorConfig{CreditAmount: 5},
+				}, nil)
+				mockStripeBackend.EXPECT().Call("POST", "/v1/products", "key_123", &stripe.ProductParams{
+					Params: stripe.Params{
+						Context: ctx,
+					},
+					ID:          new(""),
+					Name:        new(""),
+					Description: new("credit product"),
+					Metadata: map[string]string{
+						"behavior":      "per_seat",
+						"credit_amount": "5",
+						"managed_by":    "frontier",
+						"name":          "creditprod",
+						"product_id":    "creditprod",
+					},
+				}, &stripe.Product{}).Return(nil)
+				return product.NewService(stripeClient, mockProductRepo, mockPriceRepo, mockFeatureRepo)
+			},
+		},
+		{
+			name: "defaults an omitted behavior to credits on a credit product",
+			args: args{
+				product: product.Product{
+					ID:          "creditprod2",
+					Name:        "creditprod2",
+					Description: "credit product",
+					Config:      product.BehaviorConfig{CreditAmount: 5},
+				},
+			},
+			want: product.Product{
+				ID:          "creditprod2",
+				Name:        "creditprod2",
+				Description: "credit product",
+				Behavior:    product.CreditBehavior,
+				Config:      product.BehaviorConfig{CreditAmount: 5},
+			},
+			wantErr: false,
+			setup: func() *product.Service {
+				stripeClient, mockStripeBackend, mockProductRepo, mockPriceRepo, mockFeatureRepo := mockService(t)
+				mockProductRepo.EXPECT().Create(ctx, product.Product{
+					ID:          "creditprod2",
+					Name:        "creditprod2",
+					Description: "credit product",
+					Behavior:    product.CreditBehavior,
+					Config:      product.BehaviorConfig{CreditAmount: 5},
+				}).Return(product.Product{
+					ID:          "creditprod2",
+					Name:        "creditprod2",
+					Description: "credit product",
+					Behavior:    product.CreditBehavior,
+					Config:      product.BehaviorConfig{CreditAmount: 5},
+				}, nil)
+				mockStripeBackend.EXPECT().Call("POST", "/v1/products", "key_123", &stripe.ProductParams{
+					Params: stripe.Params{
+						Context: ctx,
+					},
+					ID:          new(""),
+					Name:        new(""),
+					Description: new("credit product"),
+					Metadata: map[string]string{
+						"behavior":      "credits",
+						"credit_amount": "5",
+						"managed_by":    "frontier",
+						"name":          "creditprod2",
+						"product_id":    "creditprod2",
+					},
+				}, &stripe.Product{}).Return(nil)
+				return product.NewService(stripeClient, mockProductRepo, mockPriceRepo, mockFeatureRepo)
+			},
+		},
 		{
 			name: "should create product in repo and billing provider with price and features",
 			args: args{

internal/reconcile/billingproduct.go

@@ -259,14 +259,14 @@ func billingProductChanges(s BillingProductSpec, cur currentBillingProduct) ([]s
 	if s.Description != cur.Description {
 		changes = append(changes, "description")
 	}
-	// behavior is create-only: the server sets it at create (rewriting it to
-	// "credits" when credit_amount > 0) and never changes it on update. A file
-	// that asks to change it to something the product is not cannot apply, so fail
-	// the plan rather than skip it in silence. A credit product's behavior is
-	// always "credits", so fold that in to avoid a false failure when the file
-	// still names a behavior alongside a credit amount.
+	// behavior is create-only: the server sets it at create and never changes it
+	// on update. The file's behavior is honored as written, so a file that names a
+	// behavior the product was not created with fails the plan rather than being
+	// silently overridden. Only an omitted behavior on a credit product falls back
+	// to "credits", matching the server's create-time default, so an omitted
+	// behavior does not read as a change against a credit product.
 	expectedBehavior := s.Behavior
-	if s.Config.CreditAmount > 0 {
+	if expectedBehavior == "" && s.Config.CreditAmount > 0 {
 		expectedBehavior = "credits"
 	}
 	if expectedBehavior != "" && expectedBehavior != cur.Behavior {
@@ -298,14 +298,22 @@ func billingConfigChanged(desired, cur BillingProductConfig) bool {
 		desired.MaxQuantity != cur.MaxQuantity
 }
 
+// normalizeFeatureName is the canonical form of a feature name, used by both the
+// diff and the apply so a name is compared and written the same way. The server
+// looks features up by name, so a case or whitespace difference between the two
+// would fork a duplicate feature the diff had reported as unchanged.
+func normalizeFeatureName(name string) string {
+	return strings.ToLower(strings.TrimSpace(name))
+}
+
 func billingFeatureSetsEqual(desired []BillingFeatureRef, current []string) bool {
 	d := make([]string, 0, len(desired))
 	for _, f := range desired {
-		d = append(d, strings.ToLower(strings.TrimSpace(f.Name)))
+		d = append(d, normalizeFeatureName(f.Name))
 	}
 	c := make([]string, 0, len(current))
 	for _, name := range current {
-		c = append(c, strings.ToLower(strings.TrimSpace(name)))
+		c = append(c, normalizeFeatureName(name))
 	}
 	return stringSetsEqual(uniqueSorted(d), uniqueSorted(c))
 }

internal/reconcile/billingproduct_reconciler.go

@@ -288,7 +288,9 @@ func billingProductBody(s BillingProductSpec) *frontierv1beta1.ProductRequestBod
 	}
 	features := make([]*frontierv1beta1.Feature, 0, len(s.Features))
 	for _, f := range s.Features {
-		features = append(features, &frontierv1beta1.Feature{Name: f.Name})
+		// Send the name in the same normalized form the diff compares, so the
+		// server matches an existing feature instead of forking a duplicate.
+		features = append(features, &frontierv1beta1.Feature{Name: normalizeFeatureName(f.Name)})
 	}
 	return &frontierv1beta1.ProductRequestBody{
 		Name:        s.Name,

internal/reconcile/billingproduct_test.go

@@ -149,14 +149,21 @@ func TestDiffBillingProducts(t *testing.T) {
 		}
 	})
 
-	t.Run("does not fail on a behavior a credit amount forces", func(t *testing.T) {
+	t.Run("defaults an omitted behavior to credits for a credit product", func(t *testing.T) {
 		s := newBillingProduct()
-		s.Behavior = "basic" // the file names basic, but credit_amount > 0 forces "credits"
+		s.Behavior = "" // omitted; credit_amount > 0 defaults it to the server's "credits"
 		ops, err := diffBillingProducts([]BillingProductSpec{s}, []currentBillingProduct{curToken()})
 		assert.NoError(t, err)
 		assert.Empty(t, ops)
 	})
 
+	t.Run("fails when the file names a behavior the credit product was not created with", func(t *testing.T) {
+		s := newBillingProduct()
+		s.Behavior = "basic" // the file states basic, but the product was created as credits
+		_, err := diffBillingProducts([]BillingProductSpec{s}, []currentBillingProduct{curToken()})
+		assert.ErrorContains(t, err, "behavior cannot change")
+	})
+
 	t.Run("fails the plan on a behavior change the server cannot apply", func(t *testing.T) {
 		cur := curToken()
 		cur.Behavior = "basic"
@@ -254,3 +261,17 @@ func TestDiffBillingProducts(t *testing.T) {
 		assert.Empty(t, ops)
 	})
 }
+
+func TestBillingProductBody_NormalizesFeatureNames(t *testing.T) {
+	// The diff compares feature names case-insensitively and trimmed, so the apply
+	// body must send them the same way, or the server (which looks features up by
+	// name) forks a duplicate feature the plan reported as unchanged.
+	s := newBillingProduct()
+	s.Features = []BillingFeatureRef{{Name: "  Foo  "}, {Name: "BAR"}}
+	body := billingProductBody(s)
+	var got []string
+	for _, f := range body.GetFeatures() {
+		got = append(got, f.GetName())
+	}
+	assert.Equal(t, []string{"foo", "bar"}, got)
+}