fix(api): clear manual plan caps with explicit null

internal/api/handler_plans.go

@@ -294,6 +294,12 @@ func (h *Handler) updatePlan(ctx context.Context, httpReq *events.LambdaFunction
 	if err != nil {
 		return nil, NewClientError(400, "invalid request body")
 	}
+	var capField struct {
+		MaxStepCommitment json.RawMessage `json:"max_step_commitment"`
+	}
+	if err = json.Unmarshal([]byte(httpReq.Body), &capField); err != nil {
+		return nil, NewClientError(400, "invalid request body")
+	}
 
 	// Fetch existing plan to preserve data not sent in request. Do NOT wrap
 	// err with the raw plan UUID: a non-ClientError propagates to the router's
@@ -306,7 +312,7 @@ func (h *Handler) updatePlan(ctx context.Context, httpReq *events.LambdaFunction
 			"updatePlan: GetPurchasePlan failed")
 	}
 
-	plan, err := mergeUpdatedPlan(existingPlan, &req, planID)
+	plan, err := mergeUpdatedPlan(existingPlan, &req, planID, capField.MaxStepCommitment != nil)
 	if err != nil {
 		return nil, err
 	}
@@ -325,7 +331,7 @@ func (h *Handler) updatePlan(ctx context.Context, httpReq *events.LambdaFunction
 // mergeUpdatedPlan builds the plan an update request describes, carrying over
 // the state the request cannot express (timestamps, ramp progress, services)
 // from the stored plan, and validates it.
-func mergeUpdatedPlan(existingPlan *config.PurchasePlan, req *PlanRequest, planID string) (*config.PurchasePlan, error) {
+func mergeUpdatedPlan(existingPlan *config.PurchasePlan, req *PlanRequest, planID string, capPresent bool) (*config.PurchasePlan, error) {
 	// Create new plan from request
 	plan := req.toPurchasePlan()
 	plan.ID = planID
@@ -343,9 +349,12 @@ func mergeUpdatedPlan(existingPlan *config.PurchasePlan, req *PlanRequest, planI
 		return nil, err
 	}
 
-	if req.MaxStepCommitment == nil {
+	if req.MaxStepCommitment == nil && !capPresent {
 		plan.MaxStepCommitment = existingPlan.MaxStepCommitment
 	}
+	if capPresent && req.MaxStepCommitment == nil && plan.AutoPurchase {
+		return nil, NewClientError(400, "max_step_commitment cannot be cleared while auto_purchase is enabled")
+	}
 
 	// If no services were created from request, preserve existing services
 	if len(plan.Services) == 0 && len(existingPlan.Services) > 0 {

internal/api/handler_plans_concurrency_integration_test.go

@@ -15,13 +15,16 @@ package api
 
 import (
 	"context"
+	"encoding/json"
+	"fmt"
 	"sync"
 	"testing"
 	"time"
 
 	"github.com/LeanerCloud/cloud-commitments-platform/internal/config"
 	"github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations"
 	"github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers"
+	"github.com/aws/aws-lambda-go/events"
 	"github.com/google/uuid"
 	"github.com/jackc/pgx/v5"
 	"github.com/jackc/pgx/v5/pgxpool"
@@ -262,3 +265,103 @@ func TestCreatePlannedPurchases_CanceledStepIsReschedulable(t *testing.T) {
 	require.NoError(t, err, "a step whose only rows were canceled must be reschedulable")
 	assert.Equal(t, 1, resp.Created)
 }
+
+func TestUpdatePlan_CapPresencePostgres(t *testing.T) {
+	cap100, cap5 := 100.0, 5.0
+	cases := []struct {
+		name   string
+		field  string
+		auto   bool
+		status int
+		want   *float64
+	}{
+		{"manual null clears", `,"max_step_commitment":null`, false, 200, nil},
+		{"omitted preserves", "", false, 200, &cap100},
+		{"numeric replaces", `,"max_step_commitment":5`, false, 200, &cap5},
+		{"automatic omitted preserves", "", true, 200, &cap100},
+		{"automatic numeric decreases", `,"max_step_commitment":5`, true, 200, &cap5},
+		{"automatic null refuses", `,"max_step_commitment":null`, true, 400, &cap100},
+		{"string refuses", `,"max_step_commitment":"5"`, false, 400, &cap100},
+		{"zero refuses", `,"max_step_commitment":0`, false, 400, &cap100},
+		{"negative refuses", `,"max_step_commitment":-1`, false, 400, &cap100},
+		{"precision refuses", `,"max_step_commitment":1.005`, false, 400, &cap100},
+		{"unknown remains ignored", `,"max_step_commitment":null,"future_field":false`, false, 200, nil},
+		{"case-insensitive null", `,"MAX_STEP_COMMITMENT":null`, false, 200, nil},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			ctx := context.Background()
+			f := newCreateConcurrencyFixture(ctx, t)
+			plan, err := f.store.GetPurchasePlan(ctx, f.planID)
+			require.NoError(t, err)
+			plan.Enabled, plan.AutoPurchase = true, tc.auto
+			plan.NotificationDaysBefore = 2
+			plan.MaxStepCommitment = &cap100
+			plan.Services = map[string]config.ServiceConfig{"aws:ec2": {Provider: "aws", Service: "ec2", Enabled: true, Term: 1, Payment: "no-upfront", Coverage: 80}}
+			next := time.Now().AddDate(0, 0, 7)
+			plan.NextExecutionDate = &next
+			require.NoError(t, f.store.UpdatePurchasePlan(ctx, plan))
+			before, err := f.store.GetPurchasePlan(ctx, f.planID)
+			require.NoError(t, err)
+			auth := f.handler.auth.(*MockAuthService)
+			auth.On("ValidateCSRFToken", mock.Anything, mock.Anything, mock.Anything).Return(nil)
+			body := fmt.Sprintf(`{"name":"Concurrent Create Plan","provider":"aws","service":"ec2","term":1,"payment":"no-upfront","target_coverage":80,"ramp_schedule":"weekly-25pct","enabled":true,"auto_purchase":%t,"notification_days_before":2%s}`, tc.auto, tc.field)
+			req := &events.LambdaFunctionURLRequest{
+				Headers: map[string]string{"Authorization": "Bearer admin-token", "Content-Type": "application/json", "X-CSRF-Token": "test-csrf"}, Body: body,
+				RequestContext: events.LambdaFunctionURLRequestContext{HTTP: events.LambdaFunctionURLRequestContextHTTPDescription{Method: "PUT", Path: "/api/plans/" + f.planID}},
+			}
+			response, err := f.handler.HandleRequest(ctx, req)
+			require.NoError(t, err)
+			require.Equal(t, tc.status, response.StatusCode, response.Body)
+			stored, err := f.store.GetPurchasePlan(ctx, f.planID)
+			require.NoError(t, err)
+			assert.Equal(t, tc.want, stored.MaxStepCommitment)
+			assert.Equal(t, tc.auto, stored.AutoPurchase)
+			assert.Equal(t, before.RampSchedule.CurrentStep, stored.RampSchedule.CurrentStep)
+			assert.True(t, before.RampSchedule.StartDate.Equal(stored.RampSchedule.StartDate))
+			assert.True(t, before.NextExecutionDate.Equal(*stored.NextExecutionDate))
+			req.RequestContext.HTTP.Method, req.Body = "GET", ""
+			response, err = f.handler.HandleRequest(ctx, req)
+			require.NoError(t, err)
+			require.Equal(t, 200, response.StatusCode, response.Body)
+			var returned config.PurchasePlan
+			require.NoError(t, json.Unmarshal([]byte(response.Body), &returned))
+			assert.Equal(t, tc.want, returned.MaxStepCommitment)
+		})
+	}
+}
+
+type concurrentCapStore struct{ config.StoreInterface }
+
+func (s concurrentCapStore) UpdatePurchasePlan(ctx context.Context, plan *config.PurchasePlan) error {
+	winner, err := s.GetPurchasePlan(ctx, plan.ID)
+	if err != nil {
+		return err
+	}
+	cap80 := 80.0
+	winner.MaxStepCommitment = &cap80
+	if err = s.StoreInterface.UpdatePurchasePlan(ctx, winner); err != nil {
+		return err
+	}
+	return s.StoreInterface.UpdatePurchasePlan(ctx, plan)
+}
+
+func TestUpdatePlan_CapNullPreservesConcurrentWinner(t *testing.T) {
+	ctx := context.Background()
+	f := newCreateConcurrencyFixture(ctx, t)
+	plan, err := f.store.GetPurchasePlan(ctx, f.planID)
+	require.NoError(t, err)
+	cap100 := 100.0
+	plan.MaxStepCommitment = &cap100
+	require.NoError(t, f.store.UpdatePurchasePlan(ctx, plan))
+	f.handler.auth.(*MockAuthService).On("ValidateCSRFToken", mock.Anything, mock.Anything, mock.Anything).Return(nil)
+	f.handler.config = concurrentCapStore{StoreInterface: f.store}
+	req := &events.LambdaFunctionURLRequest{Headers: map[string]string{"Authorization": "Bearer admin-token", "Content-Type": "application/json", "X-CSRF-Token": "test-csrf"}, Body: `{"name":"Concurrent Create Plan","ramp_schedule":"weekly-25pct","enabled":false,"auto_purchase":false,"max_step_commitment":null}`, RequestContext: events.LambdaFunctionURLRequestContext{HTTP: events.LambdaFunctionURLRequestContextHTTPDescription{Method: "PUT", Path: "/api/plans/" + f.planID}}}
+	response, err := f.handler.HandleRequest(ctx, req)
+	require.NoError(t, err)
+	require.Equal(t, 409, response.StatusCode, response.Body)
+	stored, err := f.store.GetPurchasePlan(ctx, f.planID)
+	require.NoError(t, err)
+	require.NotNil(t, stored.MaxStepCommitment)
+	assert.Equal(t, 80.0, *stored.MaxStepCommitment)
+}

internal/api/handler_plans_sp_multistep_test.go

@@ -49,7 +49,7 @@ func TestCreatePlan_RejectsArmedMultiStepSavingsPlan(t *testing.T) {
 func TestMergeUpdatedPlan_RejectsArmedMultiStepSavingsPlan(t *testing.T) {
 	existing := &config.PurchasePlan{Name: "p", RampSchedule: config.RampSchedule{Type: "immediate", PercentPerStep: 100, TotalSteps: 1}}
 	_, err := mergeUpdatedPlan(existing, &PlanRequest{Name: "p", Provider: "aws", Service: "savingsplans",
-		RampSchedule: "weekly-25pct", Enabled: true, AutoPurchase: true}, "id")
+		RampSchedule: "weekly-25pct", Enabled: true, AutoPurchase: true}, "id", false)
 	requireSP400(t, err)
 }
 

internal/api/handler_plans_step_cap_test.go

@@ -1,11 +1,16 @@
 package api
 
 import (
+	"context"
+	"fmt"
 	"math"
 	"testing"
 
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/auth"
 	"github.com/LeanerCloud/cloud-commitments-platform/internal/config"
+	"github.com/aws/aws-lambda-go/events"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 )
 
@@ -15,22 +20,60 @@ func TestMergeUpdatedPlan_MaxStepCommitment(t *testing.T) {
 		RampSchedule: config.RampSchedule{Type: "immediate", PercentPerStep: 100, TotalSteps: 1}}
 
 	t.Run("omitted on update keeps the current cap", func(t *testing.T) {
-		plan, err := mergeUpdatedPlan(existing, &PlanRequest{Name: "p", RampSchedule: "immediate"}, "id")
+		plan, err := mergeUpdatedPlan(existing, &PlanRequest{Name: "p", RampSchedule: "immediate"}, "id", false)
 		require.NoError(t, err)
 		require.NotNil(t, plan.MaxStepCommitment)
 		assert.Equal(t, 100.0, *plan.MaxStepCommitment)
 	})
 	t.Run("provided on update replaces it", func(t *testing.T) {
-		plan, err := mergeUpdatedPlan(existing, &PlanRequest{Name: "p", RampSchedule: "immediate", MaxStepCommitment: &cap5}, "id")
+		plan, err := mergeUpdatedPlan(existing, &PlanRequest{Name: "p", RampSchedule: "immediate", MaxStepCommitment: &cap5}, "id", true)
 		require.NoError(t, err)
 		assert.Equal(t, 5.0, *plan.MaxStepCommitment)
 	})
+	t.Run("explicit null clears a manual cap", func(t *testing.T) {
+		plan, err := mergeUpdatedPlan(existing, &PlanRequest{Name: "p", RampSchedule: "immediate"}, "id", true)
+		require.NoError(t, err)
+		assert.Nil(t, plan.MaxStepCommitment)
+		assert.Equal(t, 100.0, *existing.MaxStepCommitment)
+	})
+	t.Run("explicit null refuses automatic mode", func(t *testing.T) {
+		plan, err := mergeUpdatedPlan(existing, &PlanRequest{Name: "p", RampSchedule: "immediate", AutoPurchase: true}, "id", true)
+		require.ErrorContains(t, err, "cannot be cleared")
+		assert.Nil(t, plan)
+	})
 	for name, v := range map[string]float64{"zero": 0, "negative": -1, "NaN": math.NaN(), "Inf": math.Inf(1)} {
 		t.Run("rejects "+name, func(t *testing.T) {
 			bad := v
-			_, err := mergeUpdatedPlan(existing, &PlanRequest{Name: "p", RampSchedule: "immediate", MaxStepCommitment: &bad}, "id")
+			_, err := mergeUpdatedPlan(existing, &PlanRequest{Name: "p", RampSchedule: "immediate", MaxStepCommitment: &bad}, "id", true)
 			require.Error(t, err)
 			assert.Contains(t, err.Error(), "max_step_commitment")
 		})
 	}
 }
+
+func TestUpdatePlan_CapNullKeepsAuthorization(t *testing.T) {
+	for _, status := range []int{403, 404} {
+		t.Run(fmt.Sprint(status), func(t *testing.T) {
+			ctx := context.Background()
+			store := new(MockConfigStore)
+			mockAuth := new(MockAuthService)
+			mockAuth.On("ValidateSession", mock.Anything, "cap-token").Return(&Session{UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"}, nil)
+			mockAuth.On("ValidateCSRFToken", mock.Anything, mock.Anything, mock.Anything).Return(nil)
+			if status == 403 {
+				mockAuth.grantPermissions([]auth.Permission{{Action: auth.ActionView, Resource: auth.ResourcePlans}})
+			} else {
+				mockAuth.grantScoped("allowed-account")
+				store.GetPlanAccountsFn = func(context.Context, string) ([]config.CloudAccount, error) {
+					return []config.CloudAccount{{Name: "other-account"}}, nil
+				}
+			}
+			h := &Handler{config: store, auth: mockAuth}
+			req := &events.LambdaFunctionURLRequest{Headers: map[string]string{"Authorization": "Bearer cap-token", "Content-Type": "application/json", "X-CSRF-Token": "test-csrf"}, Body: `{"name":"p","max_step_commitment":null}`, RequestContext: events.LambdaFunctionURLRequestContext{HTTP: events.LambdaFunctionURLRequestContextHTTPDescription{Method: "PUT", Path: "/api/plans/11111111-1111-1111-1111-111111111111"}}}
+			response, err := h.HandleRequest(ctx, req)
+			require.NoError(t, err)
+			assert.Equal(t, status, response.StatusCode, response.Body)
+			store.AssertNotCalled(t, "GetPurchasePlan")
+			store.AssertNotCalled(t, "UpdatePurchasePlan")
+		})
+	}
+}

internal/api/openapi.yaml

@@ -3172,12 +3172,14 @@ components:
           type: integer
         max_step_commitment:
           type: number
+          nullable: true
           exclusiveMinimum: 0
           description: >-
             Ceiling on the total commitment (upfront + monthly * term) one ramp
             step may auto-purchase. Omit on create for no cap (auto-purchase of
             steps with recommendations is then refused); omit on update to keep
-            the current cap.
+            the current cap. On update, explicit null clears a manual plan's
+            cap and is refused while auto_purchase is true.
         provider:
           type: string
         service: